Re: [HACKERS] Standalone synchronous master

[Amit Kapila]

On Fri, Jan 10, 2014 at 9:17 PM, Bruce Momjian  wrote:
> On Fri, Jan 10, 2014 at 10:21:42AM +0530, Amit Kapila wrote:
>> Here I think if user is aware from beginning that this is the behaviour,
>> then may be the importance of message is not very high.
>> What I want to say is that if we provide a UI in such a way that user
>> decides during setup of server the behavior that is required by him.
>>
>> For example, if we provide a new parameter
>> available_synchronous_standby_names along with current parameter
>> and ask user to use this new parameter, if he wishes to synchronously
>> commit transactions on another server when it is available, else it will
>> operate as a standalone sync master.
>
> I know there was a desire to remove this TODO item, but I think we have
> brought up enough new issues that we can keep it to see if we can come
> up with a solution.

  I am not telling any such thing, rather I am suggesting some other way
  for this new mode.

> I have added a link to this discussion on the TODO
> item.
>
> I think we will need at least four new GUC variables:
>
> *  timeout control for degraded mode
> *  command to run during switch to degraded mode
> *  command to run during switch from degraded mode
> *  read-only variable to report degraded mode

Okay, this is one way of providing this new mode, others could be:

a.
Have just one GUC sync_standalone_mode = true|false and make
this as PGC_POSTMASTER parameter, so that user is only
allowed to set this mode at startup. Even if we don't want it as
Postmaster parameter, we can mention to users that they can
change this parameter only before server reaches current situation.
I understand that without any alarm or some other way, it is difficult
for user to know and change it, but I think in that case he should
set it before server startup.

b.
On above lines, instead of boolean parameter, provide a parameter
similar to current one such as available_synchronous_standby_names,
setting of this should follow what I said in point a. The benefit in this
as compare to 'a' is that it appears to be more like what we currently have.

I think if we try to solve this problem by providing a way so that user
can change it at runtime or when the problem actually occurred, it can
make the UI more complex and difficult for us to provide a way so that
user can be alerted on such situation. We can keep our options open
so that if tomorrow, we can find any reasonable way, then we can
provide it to user a mechanism for changing this at runtime, but I don't
think it is stopping us from providing a way with which user can get the
benefit of this mode by providing start time parameter.

With Regards,
Amit Kapila.
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] Compiling extensions on Windows

[Craig Ringer]

On 01/06/2014 07:44 PM, Sandeep Thakkar wrote:
> Okay.
> 
> BTW, I just checked that Windows 32bit installer ships the libintl.h.
> Did you try if it works for you? Or it requires more headers? Somehow,
> Windows 64bit installer installer missed it. I'll fix the build script. 

That appears to be the only header missing for simple compilation. I did
some testing to verify that it works.

http://blog.2ndquadrant.com/compiling-postgresql-extensions-visual-studio-windows/


-- 
 Craig Ringer   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


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

Re: [HACKERS] Compiling extensions on Windows

[Craig Ringer]

On 01/06/2014 07:44 PM, Sandeep Thakkar wrote:
> Okay.
> 
> BTW, I just checked that Windows 32bit installer ships the libintl.h.
> Did you try if it works for you? Or it requires more headers? Somehow,
> Windows 64bit installer installer missed it. I'll fix the build script. 

Actually, on second thoughts there were two other issues. One I reported
separately (double-inclusion of pg_config_os.h due to _WIN32 vs WIN32).
The other is worth looking at here.

We don't set __declspec(dllexport) on extension functions automatically
when building stand-alone on Windows. So it's necessary to explicitly
specify PGDLLEXPORT for each function. We seem to work around this in
the Perl build toolchain by forcing export of everything not explicitly
static (which is, btw, a pretty crappy thing we should revisit in favour
of using PGDLLEXPORT and -fvisibility=hidden on Linux).

Instead we should perhaps be adding this automatically via a prototype
generated by PG_FUNCTION_INFO_V1, or adding PGDLLEXPORT to all our
example documentation. I think the latter is preferable because if we
start generating a prototype for the base function in PG_FUNCTION_INFO
when we didn't before it could break existing code.

Comments?

-- 
 Craig Ringer   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


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

Re: [HACKERS] Add CREATE support to event triggers

[Simon Riggs]

On 10 January 2014 23:22, Alvaro Herrera  wrote:

> On the subject of testing the triggers, Robert Haas wrote:
>
>> Here's one idea: create a contrib module that (somehow, via APIs to be
>> invented) runs every DDL command that gets executed through the
>> deparsing code, and then parses the result and executes *that* instead
>> of the original command.  Then, add a build target that runs the
>> regression test suite in that mode, and get the buildfarm configured
>> to run that build target regularly on at least some machines.  That
>> way, adding syntax to the regular regression test suite also serves to
>> test that the deparsing logic for that syntax is working.  If we do
>> this, there's still some maintenance burden associated with having DDL
>> deparsing code, but at least our chances of noticing when we've failed
>> to maintain it should be pretty good.
>
> I gave this some more thought and hit a snag.  The problem here is that
> by the time the event trigger runs, the original object has already been
> created.  At that point, we can't simply replace the created objects
> with objects that would hypothetically be created by a command trigger.

Yes, all we are doing is firing a trigger that has access to the
information about the current object.

> A couple of very hand-wavy ideas:
>
> 1. in the event trigger, DROP the original object and CREATE it as
> reported by the creation_commands SRF.
>
> 2. Have ddl_command_start open a savepoint, and then roll it back in
> ddl_command_end, then create the object again.  Not sure this is doable
> because of the whole SPI nesting issue .. maybe with C-language event
> trigger functions?

We need to be clear what action we are testing exactly. Once we know
that we can come up with a test mechanism.

We can come up with various tests that test coverage but that may not
be enough. Mostly we're just splitting up fields from the DDL, like
object name into its own text field, as well as augmenting it with
database name. That's pretty simple and shouldn't require much
testing. This code seems likely to be prone to missing features much
more so than actual bugs. Missing a piece of useful information in the
JSON doesn't mean there is a bug.

If all the event trigger does is generate JSON, then all we need to do
is test that the trigger fired and generated well-formed JSON with the
right information content. That should be very simple and I would say
the simpler the better.
To do that we can run code to parse the JSON and produce formatted
text output like this...
Field|   Value
-+---
XX  afagaha
YY  aamnamna
The output can be derived from visual inspection, proving that we
generated the required JSON.

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


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

Re: [HACKERS] INSERT...ON DUPLICATE KEY LOCK FOR UPDATE

[Peter Geoghegan]

On Fri, Jan 10, 2014 at 7:59 PM, Peter Geoghegan  wrote:
>> *shrug*. I'm not too concerned about performance during contention. But
>> let's see how this fixed version performs. Could you repeat the tests you
>> did with this?
>
> Why would you not be too concerned about the performance with
> contention? It's a very important aspect. But even if you don't, if
> you look at the transaction throughput with only a single client in
> the update-heavy benchmark [1] (with one client there is a fair mix of
> inserts and updates), my approach still comes out far ahead.
> Transaction throughput is almost 100% higher, with the *difference*
> exceeding 150% at 8 clients but never reaching too much higher. I
> think that the problem isn't so much with contention between clients
> as much as with contention between inserts and updates, which affects
> everyone to approximately the same degree. And the average max latency
> across runs for one client is 130.447 ms, as opposed to 0.705 ms with
> my patch - that's less than 1%. Whatever way you cut it, the
> performance of my approach is far superior. Although we should
> certainly investigate the impact of your most recent revision, and I
> intend to, how can you not consider those differences to be extremely
> significant?

So I re-ran the same old benchmark, where we're almost exclusively
updating. Results for your latest revision were very similar to my
patch:

http://postgres-benchmarks.s3-website-us-east-1.amazonaws.com/exclusion-no-deadlock/

This suggests that the main problem encountered was lock contention
among old, broken promise tuples. Note that this benchmark doesn't
involve any checkpointing, and everything fits in memory.
Opportunistic pruning is possible, which I'd imagine helps a lot with
the bloat, at least in this benchmark - there are only every 100,000
live tuples. That might not always be true, of course.

In any case, my patch is bound to win decisively for the other
extreme, the insert-only case, because the overhead of doing an index
scan first is always wasted there with your approach, and the overhead
of extended btree leaf page locking has been shown to be quite low. In
the past you've spoken of avoiding that overhead through an adaptive
strategy based on statistics, but I think you'll have a hard time
beating a strategy where the decision comes as late as possible, and
is informed by highly localized page-level metadata already available.
My implementation can abort an attempt to just read an existing
would-be duplicate very inexpensively (with no strong locks), going
back to just after the _bt_search() to get a heavyweight lock if just
reading doesn't work out (if there is no duplicate found), so as to
not waste all of its prior work. Doing one of the two extremes of
insert-mostly or update-only well is relatively easy; dynamically
adapting to one or the other is much harder. Especially if it's a
consistent mix of inserts and updates, where general observations
aren't terribly useful.

All other concerns of mine still remain, including the concern over
the extra locking of the proc array - I'm concerned about the
performance impact of that on other parts of the system not exercised
by this test.

-- 
Peter Geoghegan


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

Re: [HACKERS] INSERT...ON DUPLICATE KEY LOCK FOR UPDATE

[Peter Geoghegan]

On Sat, Jan 11, 2014 at 2:39 AM, Peter Geoghegan  wrote:
> So I re-ran the same old benchmark, where we're almost exclusively
> updating. Results for your latest revision were very similar to my
> patch:
>
> http://postgres-benchmarks.s3-website-us-east-1.amazonaws.com/exclusion-no-deadlock/

To put that in context, here is a previously unpublished repeat of the
same benchmark on the slightly improved second most recently submitted
revision of mine, v6:

http://postgres-benchmarks.s3-website-us-east-1.amazonaws.com/upsert-cmp-3/

(recall that I improved things a bit by remember row-locking
conflicts, not just conflicts when we try value locking - that made a
small additional difference, reflected here but not in /upsert-cmp-2/
).

The numbers for each patch are virtually identical. I guess I could
improve my patch by not always getting a heavyweight lock on the first
insert attempt, based on the general observation that we have
previously always updated. My concern would be that that would happen
at the expense of the other case.

-- 
Peter Geoghegan


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

Re: [HACKERS] Simple improvements to freespace allocation

[Simon Riggs]

On 9 January 2014 01:38, Jim Nasby  wrote:
> On 1/8/14, 1:43 AM, Heikki Linnakangas wrote:
>
> I've wanted the cluster case for a long time. I also see the use for the
> RECENT scenario, especially if we had CLUSTER CONCURRENT that let you shrink
> the head of the table as needed.
>

> But there's probably a more important point to this one: for you to have any
> chance of packing you MUST get everything out of the tail of the table.
> Resetting to zero on every request is one possible way to do that, though it
> might be better to do something like reset only once the pointer goes past
> block X. The other thing you'd want is a way to force tuples off the last X
> pages. Due to a lack of ctid operators that was already hard, and HOT makes
> that even harder

Agreed

> (BTW, related to this you'd ideally want HOT to continue to
> operate on the front of the table, but not the back.)

That's a good idea.

> All that said, I've definitely wanted the ability to shrink tables in the
> past, though TBH I've wanted that more for indexes.
>
> Ultimately, what I really want on this front is:
>
> PACK TABLE blah BACKGROUND;
> CLUSTER TABLE blah BACKGROUND;
> REINDEX INDEX blah BACKGROUND;
>
> where BACKGROUND would respect a throttle setting. (While I'm dreaming, it'd
> be nice to have DATABASE/TABLESPACE/SCHEMA alternate specifications too...)

I like the idea of declarative deprioritisation.

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


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

Re: [HACKERS] array_length(anyarray)

[Florian Pflug]

On Jan10, 2014, at 15:10 , Merlin Moncure  wrote:
> On Fri, Jan 10, 2014 at 6:00 AM, Florian Pflug  wrote:
>> On Jan10, 2014, at 11:00 , Merlin Moncure  wrote:
>>> On Fri, Jan 10, 2014 at 3:52 AM, Marko Tiikkaja  wrote:
 On 1/10/14, 10:41 AM, Merlin Moncure wrote:
> 
> What's needed for better iteration support (IMO)
> is a function that does what unnest does but returns an array on
> indexes (one per dimsension) -- a generalization of the
> _pg_expandarray function.  Lets' say 'unnest_dims'.
 
 
 So  unnest_dims('{{1,2},{3,4}}'::int[])  would return  VALUES (1,
 '{1,2}'::int[]), (2, '{3,4}'::int[])?  If so, then yes, that's a
 functionality I've considered us to have been missing for a long time.
>>> 
>>> not quite.  it returns int[], anyelement: so, using your example, you'd get:
>>> 
>>> [1,1], 1
>>> [1,2], 2
>>> [2,1], 3
>>> [2,2], 4
>> 
>> Now that we have WITH ORDINALITY, it'd be sufficient to have a
>> variant of array_dims() that returns int[][] instead of text, say
>> array_dimsarray(). Your unnest_dims could then be written as
>> 
>>  unnest(array_dimsarray(array)) with ordinality
> 
> hm, not quite following that.  maybe an example?
> 
> my issue with 'WITH ORDINALITY' (while it's pretty neat) is that it
> doesn't give you the dimension coordinate of each datum so you can't
> really use it to slice.  with unnest_dims(), you an slice, say via:

Sorry, I misunderstood what you were proposing. I though you intended
unnest_dims to returns one row per dimension, containing the index and
bounds of that dimension. And yeah, that fact your your mail showed
unnest_dims returning *4* rows for a *2*-dimensional array should have
tipped me off. 

best regards,
Florian Pflug



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

Re: [HACKERS] Compiling extensions on Windows

[Tom Lane]

Craig Ringer  writes:
> We don't set __declspec(dllexport) on extension functions automatically
> when building stand-alone on Windows. So it's necessary to explicitly
> specify PGDLLEXPORT for each function.

I'm not sure I believe this.  I don't see any PGDLLEXPORT symbols in any
of the standard contrib modules; how is it that they work?

> Instead we should perhaps be adding this automatically via a prototype
> generated by PG_FUNCTION_INFO_V1, or adding PGDLLEXPORT to all our
> example documentation. I think the latter is preferable because if we
> start generating a prototype for the base function in PG_FUNCTION_INFO
> when we didn't before it could break existing code.

> Comments?

One of the things I've always found particularly vile about Microsoft
is the way that they seem to think it's fine to make people sprinkle
Windows-only droppings throughout code that's supposed to be portable.
I'm not in favor of asking people to write out PGDLLEXPORT manually
on every function unless it's *absolutely* necessary, and the available
evidence suggests to me that it isn't.

So if it's really necessary to change anything here, I'd rather see us
take the approach of hiding it in PG_FUNCTION_INFO_V1.  What happens
if we do that and there's also a manually-written prototype?

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] Standalone synchronous master

[Bruce Momjian]

On Sat, Jan 11, 2014 at 01:29:23PM +0530, Amit Kapila wrote:
> Okay, this is one way of providing this new mode, others could be:
> 
> a.
> Have just one GUC sync_standalone_mode = true|false and make
> this as PGC_POSTMASTER parameter, so that user is only
> allowed to set this mode at startup. Even if we don't want it as
> Postmaster parameter, we can mention to users that they can
> change this parameter only before server reaches current situation.
> I understand that without any alarm or some other way, it is difficult
> for user to know and change it, but I think in that case he should
> set it before server startup.
> 
> b.
> On above lines, instead of boolean parameter, provide a parameter
> similar to current one such as available_synchronous_standby_names,
> setting of this should follow what I said in point a. The benefit in this
> as compare to 'a' is that it appears to be more like what we currently have.
> 
> I think if we try to solve this problem by providing a way so that user
> can change it at runtime or when the problem actually occurred, it can
> make the UI more complex and difficult for us to provide a way so that
> user can be alerted on such situation. We can keep our options open
> so that if tomorrow, we can find any reasonable way, then we can
> provide it to user a mechanism for changing this at runtime, but I don't
> think it is stopping us from providing a way with which user can get the
> benefit of this mode by providing start time parameter.

I am not sure how this would work.  Right now we wait for one of the
synchronous_standby_names servers to verify the writes.   We need some
way of telling the system how long to wait before continuing in degraded
mode.  Without a timeout and admin notification, it doesn't seem much
better than our async mode, which is what many people were complaining
about.

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

  + Everyone has their own god. +


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

Re: [HACKERS] Compiling extensions on Windows

[Magnus Hagander]

On Sat, Jan 11, 2014 at 5:00 PM, Tom Lane  wrote:

> Craig Ringer  writes:
> > We don't set __declspec(dllexport) on extension functions automatically
> > when building stand-alone on Windows. So it's necessary to explicitly
> > specify PGDLLEXPORT for each function.
>
> I'm not sure I believe this.  I don't see any PGDLLEXPORT symbols in any
> of the standard contrib modules; how is it that they work?
>

They are built through our perl toolkit, which enables exporting of *all*
symbols, regardless of flags in the code.

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

Re: [HACKERS] [PATCH] pg_basebackup: progress report max once per second

[Magnus Hagander]

On Fri, Jan 10, 2014 at 3:56 AM, Mika Eloranta  wrote:

> On Thu, Jan 9, 2014, at 20:05, Magnus Hagander wrote:
> > On Thu, Nov 14, 2013 at 10:27 AM, Mika Eloranta  wrote:
> >> On 13 Nov 2013, at 20:51, Mika Eloranta  wrote:
> >>
> >>
> >>
> > Prevent excessive progress reporting that can grow to gigabytes
> >>
> > of output with large databases.
> >>
> >>
> >> Same patch as an attachment.
> >
> > Would it not make more sense to instead store the last number printed,
> and only print it if the percentage has changed? AIUI with this patch we
> still print the same thing on top of itself if it takes >1 second to get 1%
> further.
> >
> > (Except for verbose mode - but if you're asking for verbose mode, you
> are *asking* to get lots of output)
>
> (re-sent response as I used the wrong sender address previously, sorry
> about the dupe)
>
> Printing out progress periodically is IMHO slightly better as the
> interactive user can see that there is some progress (e.g. by pressing
> enter for a new empty console line) during a huge basebackup operation.
>

That's an argument I hadn't considered - but I still think it's acceptable
to wait until the next percentage digit in this case.


The original problem I wanted to address was that I had a daemon
> watching over the basebackup process and reading its output to make sure
> that the basebackup is proceeding properly. It also wrote all the output
> to a logfile for postmortem analysis. The log file grew to a very big
> size (multiple gigabytes due to the progress prints). With my patch the
> log was only a few kilos, without any negative effects. The excessive
> progress reporting can also be an issue in an interactive session over a
> slow network (mobile), hogging both time and bandwidth.
>

Yeah, I was guessing it was something like that. I didn't realize you'd
actually monitor it through a deamon (I've just looked at the output
filesize when minitoring things like that), but the remote connection was
easily reproducible. I definitely agree we should do something about it.


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

Re: [HACKERS] Standalone synchronous master

[Florian Pflug]

On Jan11, 2014, at 01:48 , Joshua D. Drake  wrote:
> On 01/10/2014 04:38 PM, Stephen Frost wrote:
>> Adrian,
>> 
>> * Adrian Klaver (adrian.kla...@gmail.com) wrote:
>>> On 01/10/2014 04:25 PM, Stephen Frost wrote:
 * Adrian Klaver (adrian.kla...@gmail.com) wrote:
> A) Change the existing sync mode to allow the master and standby
> fall out of sync should a standby fall over.
 
 I'm not sure that anyone is argueing for this..
>>> 
>>> Looks like here, unless I am really missing the point:
>> 
>> Elsewhere in the thread, JD agreed that having it as an independent
>> option was fine.
> 
> Yes. I am fine with an independent option.

Hm, I was about to suggest that you can set statement_timeout before
doing COMMIT to limit the amount of time you want to wait for the
standby to respond. Interestingly, however, that doesn't seem to work,
which is weird, since AFAICS statement_timeout simply generates a
query cancel requester after the timeout has elapsed, and cancelling
the COMMIT with Ctrl-C in psql *does* work.

I'm quite probably missing something, but what?

best regards,
Florian Pflug



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

Re: [HACKERS] Standalone synchronous master

[Tom Lane]

Florian Pflug  writes:
> Hm, I was about to suggest that you can set statement_timeout before
> doing COMMIT to limit the amount of time you want to wait for the
> standby to respond. Interestingly, however, that doesn't seem to work,
> which is weird, since AFAICS statement_timeout simply generates a
> query cancel requester after the timeout has elapsed, and cancelling
> the COMMIT with Ctrl-C in psql *does* work.

> I'm quite probably missing something, but what?

finish_xact_command() disables statement timeout before committing.

Not sure about the pros and cons of doing that later in the sequence.

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] Standalone synchronous master

[Andres Freund]

On 2014-01-11 18:28:31 +0100, Florian Pflug wrote:
> Hm, I was about to suggest that you can set statement_timeout before
> doing COMMIT to limit the amount of time you want to wait for the
> standby to respond. Interestingly, however, that doesn't seem to work,
> which is weird, since AFAICS statement_timeout simply generates a
> query cancel requester after the timeout has elapsed, and cancelling
> the COMMIT with Ctrl-C in psql *does* work.

I think that'd be a pretty bad API since you won't know whether the
commit failed or succeeded but replication timed out. There very well
might have been longrunning constraint triggers or such taking a long
time.
So it really would need a separate GUC.

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


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

Re: [HACKERS] Compiling extensions on Windows

[Tom Lane]

Magnus Hagander  writes:
> On Sat, Jan 11, 2014 at 5:00 PM, Tom Lane  wrote:
>> I'm not sure I believe this.  I don't see any PGDLLEXPORT symbols in any
>> of the standard contrib modules; how is it that they work?

> They are built through our perl toolkit, which enables exporting of *all*
> symbols, regardless of flags in the code.

That seems like a perfectly reasonable solution, given the way we use
loadable modules.  Excess symbols in the module shouldn't really do
any harm.  Can't we just document the flags to use for this, if you're
building in some other way?

regards, tom lane


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

Re: [HACKERS] pg_export_snapshot on standby side

[Bruce Momjian]

Uh, was this ever addressed?  I don't think so.

---

On Sat, May 25, 2013 at 10:18:57AM +0100, Simon Riggs wrote:
> On 21 May 2013 19:16, Fujii Masao  wrote:
> 
> > We cannot run parallel pg_dump on the standby server because
> > pg_export_snapshot() always fails on the standby. Is this the oversight
> > of parallel pg_dump or pg_export_snapshot()?
> >
> > pg_export_snapshot() fails in the standby because it always assigns
> > new XID and which is not allowed in the standby. Do we really need
> > to assign new XID even in the standby for the exportable snapshot?
> 
> Having looked at the code, I say No, we don't *need* to.
> 
> There are various parts of the code that deal with
> takenDuringRecovery, so much of this was clearly intended to work in
> recovery.
> 
> We use the topXid for the name of the snapshot file. That is clearly
> unnecessary and we should be using the virtualxid instead like we do
> elsewhere. We also use the topXid to test whether it is still running,
> though again, we could equally use the virtualxid instead. There is no
> problem with virtualxids possibly not being active anymore, since if
> we didn't have an xid before and don't have one now, and the xmin is
> the same, the snapshot is still valid.
> 
> I think we should treat this as a bug and fix it in 9.3 while we're
> still in beta. Why? Because once we begin using the topXid as the
> filename we can't then change later to using the vxid.
> 
> --
>  Simon Riggs   http://www.2ndQuadrant.com/
>  PostgreSQL Development, 24x7 Support, Training & Services
> 
> 
> -- 
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers

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

  + Everyone has their own god. +


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

Re: [HACKERS] [GENERAL] pg_upgrade & tablespaces

[Bruce Momjian]

On Sat, Jan 11, 2014 at 10:40:20AM -0800, Adrian Klaver wrote:
> >Right.  I know there were multiple issue with this upgrade, jails
> >probably being the biggest, but a new one I had never heard is that _if_
> >you are placing your tablespaces in the PGDATA directory, and you are
> >upgrading from pre-9.2, if you rename the old data directory, you also
> >need to start the old server and update pg_tablespace.spclocation.
> >
> >No one has ever reported that failure, but it would certainly happen.  I
> >wonder if pg_upgrade should be modified to check that
> >pg_tablespace.spclocation point to real directories for pre-9.2 servers.
> >
> 
> I thought I was understanding, now I am not. This starts with your
> post of last night. So in pre-9.2 cases the tablespace location is
> recorded in two places pg_tablespace and the symlinks in pg_tblspc/.

[ I am moving this discussion to hackers to get developer feedback. ]

Right.

> When you upgrade pg_upgrade only looks at the pg_tablespace  entry
> for pre-9.2 instances or does it look at the pg_tblspc symlinks
> also? If it looks at the symlinks would they need to be changed
> also?

pg_upgrade looks in the pg_tablespace in pre-9.2, and uses a function in
9.2+.  The query is:

snprintf(query, sizeof(query),
 "SELECT%s "
 "FROM  pg_catalog.pg_tablespace "
 "WHERE spcname != 'pg_default' AND "
 "  spcname != 'pg_global'",
/* 9.2 removed the spclocation column */
 (GET_MAJOR_VERSION(old_cluster.major_version) <= 901) ?
--> "spclocation" : "pg_catalog.pg_tablespace_location(oid) AS spclocation");

> As to your check for directories that sounds like a good idea,
> though I have one question. What constitutes a 'real' directory? I
> can see a situation where someone moves an existing instance from
> $PGDATA to $PGDATA.old and the installs a new version in $PGDATA.
> Then before they do the upgrade they create a new tablespace
> directory in $PGDATA. If they did not upgrade the spclocation in the
> old instance and ran the check it would find a directory but there
> would be nothing in it. So would the check look for actual
> tablespace data?

I would probably just look for the directory.  People are not supposed
to be modifying their clusters during the upgrade, though, as stated, if
they move the old cluster, the are required to update pg_tablespace if
they have tablespaces in PGDATA, which is unfortunate.

I think the big question on adding a check is, how many users of 9.4 are
going to be upgrading from pre-9.2 and have tablespaces in PGDATA, and
will be renaming their old PGDATA directory during the upgrade?  We
could add the test to 9.3 too, of course.

Having pg_tablespace and the symlinks get out of sync was the reason
Magnus removed that duplication in 9.2, but I was unaware of how
pg_upgrade really magnifies the problem for tablespaces in PGDATA by
recommending a PGDATA rename.

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

  + Everyone has their own god. +


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

Re: [HACKERS] Compiling extensions on Windows

[Magnus Hagander]

On Sat, Jan 11, 2014 at 7:05 PM, Tom Lane  wrote:

> Magnus Hagander  writes:
> > On Sat, Jan 11, 2014 at 5:00 PM, Tom Lane  wrote:
> >> I'm not sure I believe this.  I don't see any PGDLLEXPORT symbols in any
> >> of the standard contrib modules; how is it that they work?
>
> > They are built through our perl toolkit, which enables exporting of *all*
> > symbols, regardless of flags in the code.
>
> That seems like a perfectly reasonable solution, given the way we use
> loadable modules.  Excess symbols in the module shouldn't really do
> any harm.  Can't we just document the flags to use for this, if you're
> building in some other way?
>

It's not a build flag, and that's the main problem. It's the src/tools/msvc/
gendef.pl script that builds the export list. And what Craig is after here
is being able to build extensions using standard tools without needing our
full build infrastructure.

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

Re: [HACKERS] units in postgresql.conf comments

[Bruce Momjian]

On Wed, May 29, 2013 at 09:59:10PM -0400, Peter Eisentraut wrote:
> I think these sort of entries don't make much sense:
> 
> #wal_sender_timeout = 60s  # in milliseconds; 0 disables
> 
> I think we should remove units from the comments when it's clear from
> the name or the default value that time units are accepted.

So, is anyone doing this?  Should it be a TODO item?

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

  + Everyone has their own god. +


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

Re: [HACKERS] Partitioning performance: cache stringToNode() of pg_constraint.ccbin

[Bruce Momjian]

On Mon, Jun  3, 2013 at 03:07:27PM -0400, Noah Misch wrote:
> A colleague, Korry Douglas, observed a table partitioning scenario where
> deserializing pg_constraint.ccbin is a hot spot.  The following test case, a
> simplification of a typical partitioning setup, spends 28% of its time in
> stringToNode() and callees thereof:

Noah, what is the status on this?

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

  + Everyone has their own god. +


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

Re: [HACKERS] Recovery to backup point

[Peter Eisentraut]

The documentation doesn't build.



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

Re: [HACKERS] units in postgresql.conf comments

[Tom Lane]

Bruce Momjian  writes:
> On Wed, May 29, 2013 at 09:59:10PM -0400, Peter Eisentraut wrote:
>> I think these sort of entries don't make much sense:
>> 
>> #wal_sender_timeout = 60s  # in milliseconds; 0 disables
>> 
>> I think we should remove units from the comments when it's clear from
>> the name or the default value that time units are accepted.

> So, is anyone doing this?  Should it be a TODO item?

I think Peter's wrong here, for two reasons:

* The comment tells you what undecorated "wal_sender_timeout = 60" will
do.

* The comment tells you what the precision of the setting is.  For
instance, archive_timeout is in seconds; you can try setting it to "10ms"
if you like, but that won't do much for you.

We could imagine making these points moot, by disallowing inputs that lack
units and converting all time GUCs into some common scale (requiring
wider-than-int storage) ... but that seems sufficiently non backward
compatible that I don't see it happening.  It's not clear that it'd be a
usability improvement anyway.

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

[Peter Eisentraut]

The documentation doesn't build.




-- 
Sent 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: make_timestamp function

[Tomas Vondra]

Hi,

I've done a quick review of this patch:

1) patch applies fine to the current HEAD, with a few hunks offset
   by a few lines

2) the compilation fails because of duplicate OIDs in pg_proc, so
   I had to change 3969-3975 to 4033-4039, then it compiles fine

3) make installcheck works fine

4) No regression tests for make_time / make_date.

5) The documentation is incomplete - make_date / make_time are missing.

6) The documentation should mention that when the 'timezone' parameter
   is not set explicitly, the current timezone is used.

7) Why do the functions accept only the timezone abbreviation, not the
   full name? I find it rather confusing, because the 'timezone' option
   uses the full name, and we're using this as the default. But doing
   'show timestamp' and using the returned value fails. Is it possible
   to fix this somehow?



regards
Tomas


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

Re: [HACKERS] nested hstore patch

[Erik Rijkers]

On Sat, January 11, 2014 20:30, Peter Eisentraut wrote:
> The documentation doesn't build.

corrective patch is here:

http://www.postgresql.org/message-id/37b9f104d5a838eec9b75f3668517aa5.squir...@webmail.xs4all.nl







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

Re: [HACKERS] pg_export_snapshot on standby side

[Simon Riggs]

On 11 January 2014 18:34, Bruce Momjian  wrote:
>
> Uh, was this ever addressed?  I don't think so.

It appears not.

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


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

Re: [HACKERS] Deprecations in authentication

[Peter Eisentraut]

On Thu, 2013-10-24 at 20:37 +0200, Magnus Hagander wrote:
> On Thu, Oct 24, 2013 at 8:35 PM, Peter Eisentraut 
> wrote:
> > On 10/18/12, 7:20 AM, Magnus Hagander wrote:
> >> 1. krb5 authentication. We've had gssapi since 8.3 (which means in
> all
> >> supported versions). krb5 has been deprecated, also since 8.3. Time
> to
> >> remove it?
> >
> > OS X Mavericks has now marked just about everything in krb5.h as
> > deprecated, leading to compiler warnings.  Which reminded me of this
> > thread.  Maybe it's time.
> 
> Yeah, it's still sitting on my TODO to get done for 9.4. I guess
> that's another reason...

Are you still planning to do this?



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

Re: [HACKERS] [GENERAL] pg_upgrade & tablespaces

[Adrian Klaver]

On 01/11/2014 10:55 AM, Bruce Momjian wrote:

On Sat, Jan 11, 2014 at 10:40:20AM -0800, Adrian Klaver wrote:

Right.  I know there were multiple issue with this upgrade, jails
probably being the biggest, but a new one I had never heard is that _if_
you are placing your tablespaces in the PGDATA directory, and you are
upgrading from pre-9.2, if you rename the old data directory, you also
need to start the old server and update pg_tablespace.spclocation.

No one has ever reported that failure, but it would certainly happen.  I
wonder if pg_upgrade should be modified to check that
pg_tablespace.spclocation point to real directories for pre-9.2 servers.



I thought I was understanding, now I am not. This starts with your
post of last night. So in pre-9.2 cases the tablespace location is
recorded in two places pg_tablespace and the symlinks in pg_tblspc/.


[ I am moving this discussion to hackers to get developer feedback. ]

Right.


When you upgrade pg_upgrade only looks at the pg_tablespace  entry
for pre-9.2 instances or does it look at the pg_tblspc symlinks
also? If it looks at the symlinks would they need to be changed
also?


pg_upgrade looks in the pg_tablespace in pre-9.2, and uses a function in
9.2+.  The query is:

 snprintf(query, sizeof(query),
  "SELECT%s "
  "FROM  pg_catalog.pg_tablespace "
  "WHERE spcname != 'pg_default' AND "
  "  spcname != 'pg_global'",
 /* 9.2 removed the spclocation column */
  (GET_MAJOR_VERSION(old_cluster.major_version) <= 901) ?
--> "spclocation" : "pg_catalog.pg_tablespace_location(oid) AS spclocation");



I see, though I have another question. If pg_tablespace and the symlinks 
can get out of sync, as you say below, why is pg_tablespace considered 
the authority? Or to put it another way, why not just look at the 
symlinks as in 9.2+?





As to your check for directories that sounds like a good idea,
though I have one question. What constitutes a 'real' directory? I
can see a situation where someone moves an existing instance from
$PGDATA to $PGDATA.old and the installs a new version in $PGDATA.
Then before they do the upgrade they create a new tablespace
directory in $PGDATA. If they did not upgrade the spclocation in the
old instance and ran the check it would find a directory but there
would be nothing in it. So would the check look for actual
tablespace data?


I would probably just look for the directory.  People are not supposed
to be modifying their clusters during the upgrade, though, as stated, if
they move the old cluster, the are required to update pg_tablespace if
they have tablespaces in PGDATA, which is unfortunate.

I think the big question on adding a check is, how many users of 9.4 are
going to be upgrading from pre-9.2 and have tablespaces in PGDATA, and
will be renaming their old PGDATA directory during the upgrade?  We
could add the test to 9.3 too, of course.


Well it is not generally accepted that users should even be creating 
tablespaces in $PGDATA, but it is allowed by the program. My inclination 
is to say that it is then the programs'(Postgres) responsibility to deal 
with it. The alternative is to clarify the documentation and make it the 
users responsibility. As to users upgrading from 9.1- to 9.2+, I see 
still a lot of users posting to --general using 9.1- versions. At some 
point they will likely migrate, so I can see a fix being worthwhile.




Having pg_tablespace and the symlinks get out of sync was the reason
Magnus removed that duplication in 9.2, but I was unaware of how
pg_upgrade really magnifies the problem for tablespaces in PGDATA by
recommending a PGDATA rename.



Well it was based on the valid assumption that people would create new 
tablespaces outside $PGDATA because that is really is what is meant to 
happen. You know us users we like to test assumptions:)


--
Adrian Klaver
adrian.kla...@gmail.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] Standalone synchronous master

[Mark Kirkwood]

On 11/01/14 13:25, Stephen Frost wrote:

Adrian,


* Adrian Klaver (adrian.kla...@gmail.com) wrote:

A) Change the existing sync mode to allow the master and standby
fall out of sync should a standby fall over.


I'm not sure that anyone is argueing for this..


B) Create a new mode that does this without changing the existing sync mode.

My two cents would be to implement B. Sync to me is a contract that
master and standby are in sync at any point in time. Anything else
should be called something else. Then it is up to the documentation
to clearly point out the benefits/pitfalls. If you want to implement
something as important as replication without reading the docs then
the results are on you.


The issue is that there are folks who are argueing, essentially, that
"B" is worthless, wrong, and no one should want it and therefore we
shouldn't have it.



We have some people who clearly do want it (and seemed to have provided 
sensible arguments about why it might be worthwhile), and the others who 
say they should not.


My 2c is:

The current behavior in CAP theorem speak is 'Cap' - i.e focused on 
consistency at the expense of availability. A reasonable thing to want.


The other behavior being asked for is 'cAp' - i.e focused on 
availability. Also a reasonable configuration to want. Now the desire to 
use sync rather than async is to achieve as much consistency as 
possible, which is also reasonable.


I think an option to control whether we operate 'Cap' or 'cAp' 
(defaulting to the current 'Cap' I guess) is probably the best solution.


Regards

Mark



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

Re: [HACKERS] nested hstore patch

[Andrew Dunstan]

On 01/11/2014 03:03 PM, Erik Rijkers wrote:

On Sat, January 11, 2014 20:30, Peter Eisentraut wrote:

The documentation doesn't build.

corrective patch is here:

http://www.postgresql.org/message-id/37b9f104d5a838eec9b75f3668517aa5.squir...@webmail.xs4all.nl



It's been committed at 
. 
It will be in the next version of the patch posted.


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] Compiling extensions on Windows

[Andrew Dunstan]

On 01/11/2014 01:55 PM, Magnus Hagander wrote:
On Sat, Jan 11, 2014 at 7:05 PM, Tom Lane > wrote:


Magnus Hagander mailto:mag...@hagander.net>>
writes:
> On Sat, Jan 11, 2014 at 5:00 PM, Tom Lane mailto:t...@sss.pgh.pa.us>> wrote:
>> I'm not sure I believe this.  I don't see any PGDLLEXPORT
symbols in any
>> of the standard contrib modules; how is it that they work?

> They are built through our perl toolkit, which enables exporting
of *all*
> symbols, regardless of flags in the code.

That seems like a perfectly reasonable solution, given the way we use
loadable modules.  Excess symbols in the module shouldn't really do
any harm.  Can't we just document the flags to use for this, if you're
building in some other way?


It's not a build flag, and that's the main problem. It's the 
src/tools/msvc/gendef.pl  script that builds the 
export list. And what Craig is after here is being able to build 
extensions using standard tools without needing our full build 
infrastructure.






What I'd like is something that would use or mimic our msvc build tools 
but for extensions. (And no, I don't have time to build it.)


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] Standalone synchronous master

[Tom Lane]

Mark Kirkwood  writes [slightly rearranged]
> My 2c is:

> The current behavior in CAP theorem speak is 'Cap' - i.e focused on 
> consistency at the expense of availability. A reasonable thing to want.

> The other behavior being asked for is 'cAp' - i.e focused on 
> availability. Also a reasonable configuration to want.

> I think an option to control whether we operate 'Cap' or 'cAp' 
> (defaulting to the current 'Cap' I guess) is probably the best solution.

The above is all perfectly reasonable.  The argument that's not been made
to my satisfaction is that the proposed patch is a good implementation of
'cAp'-optimized behavior.  In particular,

> ... Now the desire to 
> use sync rather than async is to achieve as much consistency as 
> possible, which is also reasonable.

I don't think that the existing sync mode is designed to do that, and
simply lobotomizing it as proposed doesn't get you there.  I think we
need a replication mode that's been designed *from the ground up*
with cAp priorities in mind.  There may end up being only a few actual
differences in behavior --- but I fear that some of those differences
will be crucial.

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

[David E. Wheeler]

On Jan 11, 2014, at 1:47 PM, Andrew Dunstan  wrote:

> It's been committed at 
> .
>  It will be in the next version of the patch posted.

Bah! Sorry about that. Habit from decades of typing HTML.

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] Syntax of INSERT...ON DUPLICATE KEY LOCK FOR UPDATE

[Peter Geoghegan]

Someone suggested to me that I solicit opinion on the chosen syntax of
INSERT...ON DUPLICATE KEY LOCK FOR UPDATE on a separate thread. I'll
do that here, while also giving a formal user-level definition of the
feature. I'd like to solicit feedback from a wider set of people than
those participating in the main thread, while avoiding talking about
arcane details around locking which have dominated discussions up
until this point.

The patch proposed adds an optional ON DUPLICATE KEY LOCK FOR UPDATE
clause. It also extends the RETURNING clause to support projecting
"REJECTS".

Idiomatic wCTE usage, through which it's possible to build an
"UPSERT", might look like:

CREATE TABLE foo
(
  a int4 PRIMARY KEY,
  b int4,
  c text
);

WITH rej AS (
INSERT INTO foo (a, b, c)
-- Multiple rows proposed for insertion - may insert or update each
VALUES (44, 1078, 'insert'), (55, 1088, 'insert')
ON DUPLICATE KEY LOCK FOR UPDATE
RETURNING REJECTS a)
UPDATE foo SET c = 'update' FROM rej WHERE foo.a = rej.a;

This has what I like to call the fundamental upsert property: You
always either get an insert, or an update. People are already
incorrectly using wCTEs like this; I thought I'd make it work
correctly.

I believe that we'll be well served by the flexibility of what I've
proposed. In many respects it is similar to SQL MERGE. For example, I
could have deleted rather than updated, or I could have had a more
complex predicate in the update clause, so that updates did not occur
on all rejecting rows. While I'm not particularly attached to the
syntax, I do think that the flexibility is a key strength.

I recently suggested that rather than RETURNING REJECTS, we could have
a REJECTING clause, which would see a DML statement project strictly
the complement of what RETURNING projects in the same context. So
perhaps you could also see what RETURNING would not have projected
because a before row trigger returned NULL (i.e. when a before trigger
indicates to not proceed with insertion). That is certainly more
general, and so is perhaps preferable. It's also less verbose, and it
seems less likely to matter that we'll need to make REJECTING a fully
reserved keyword, as compared to REJECTS. (RETURNING is already a
fully reserved keyword not described by the standard, so this makes a
certain amount of sense to me). If nothing else, REJECTING is more
terse than RETURNING REJECTS.

Like the MySQL feature "INSERT...ON DUPLICATE KEY UPDATE", duplicates
are defined as would-be violators of any and all unique indexes. Like
the MySQL feature, the user is generally obligated to make sure they
know ahead of time which unique index any violation may come from, or
else they may update the wrong row (i.e. the row they ultimately
update is not already locked). Unlike the MySQL feature, expert users
have some capacity to recover if that problem is anticipated, because
we also may project out the rejecting row's ctid.

Notably the wCTE pattern, with a ctid join condition doesn't work very
well, because, as src/backend/optimizer/path/tidpath.c says:

 * There is currently no special support for joins involving CTID; in
 * particular nothing corresponding to best_inner_indexscan().  Since it's
 * not very useful to store TIDs of one table in another table, there
 * doesn't seem to be enough use-case to justify adding a lot of code
 * for that.

You end up with a seqscan, not a tidscan, so I don't think every
novice user is going to try this as a premature optimization, without
appreciating the hazards of tid updates. Projecting the rejecting
row's tid is an expert level feature, added mostly with things like
multi-master replication conflict resolution in mind. Those use-cases
will find this feature quite important, and will particularly value
the flexibility. In fact, I think for that use-case, it's even more
useful than SQL MERGE for a couple of reasons, in particular the
capability to defer doing anything with the locked row until later
commands.

This feature is not supposed to fully satisfy those calling for SQL
MERGE. I anticipate that we'll still get MERGE in a future release,
and the implementation anticipates this as well.

More formally, what the feature does is:

* Ensure that a row is either inserted successfully, or that if an
effort to do so was unsuccessful, the first conclusively committed
tuple with a conflicting value is exclusive locked. Not all
conflicting tuples are locked, just the first, although the order in
which we check unique indexes for conflicts is well defined (at least
with my implementation, where when we find a conflicting TID, it must
really be the first one at that juncture, because all previous unique
indexes are value locked).

* Ensure that in READ COMMITTED mode, the locked row is always
visible. Since we can get a lock on the tuple blamed for insertion not
proceeding, it must be conclusively committed and not updated or
deleted by anyone else, but in and of itself that isn't sufficient.
The tuple's xact may logi

Re: [HACKERS] Race condition in b-tree page deletion

[Peter Geoghegan]

On Sat, Dec 21, 2013 at 5:50 AM, Peter Eisentraut  wrote:
> This patch didn't make it out of the 2013-11 commit fest.  You should
> move it to the next commit fest (probably with an updated patch)
> before January 15th, if it is not resolved before then.

Uh, it's already in the January commitfest.

-- 
Peter Geoghegan


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

Re: [HACKERS] Standalone synchronous master

[Josh Berkus]

On 01/10/2014 06:27 PM, Bruce Momjian wrote:
> How would that work?  Would it be a tool in contrib?  There already is a
> timeout, so if a tool checked more frequently than the timeout, it
> should work.  The durable notification of the admin would happen in the
> tool, right?

Well, you know what tool *I'm* planning to use.

Thing is, when we talk about auto-degrade, we need to determine things
like "Is the replica down or is this just a network blip"? and take
action according to the user's desired configuration.  This is not
something, realistically, that we can do on a single request.  Whereas
it would be fairly simple for an external monitoring utility to do:

1. decide replica is offline for the duration (several poll attempts
have failed)

2. Send ALTER SYSTEM SET to the master and change/disable the
synch_replicas.

Such a tool would *also* be capable of detecting when the synchronous
replica was back up and operating, and switch back to sync mode,
something we simply can't do inside Postgres.  And it would be a lot
easier to configure an external tool with monitoring system integration
so that it can alert the DBA to degradation in a way which the DBA was
liable to actually see (which is NOT the Postgres log).

In other words, if we're going to have auto-degrade, the most
intelligent place for it is in
RepMgr/HandyRep/OmniPITR/pgPoolII/whatever.  It's also the *easiest*
place.  Anything we do *inside* Postgres is going to have a really,
really hard time determining when to degrade.


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


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

Re: [HACKERS] units in postgresql.conf comments

[Josh Berkus]

On 01/11/2014 11:06 AM, Bruce Momjian wrote:
> On Wed, May 29, 2013 at 09:59:10PM -0400, Peter Eisentraut wrote:
>> I think these sort of entries don't make much sense:
>>
>> #wal_sender_timeout = 60s  # in milliseconds; 0 disables
>>
>> I think we should remove units from the comments when it's clear from
>> the name or the default value that time units are accepted.
> 
> So, is anyone doing this?  Should it be a TODO item?

I don't agree, actually, unless we take the next step and actually clean
all the documentation garbage out of the file and leave it in the main
docs and pg_settings where it belongs.


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


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

Re: [HACKERS] Recovery to backup point

[MauMau]

From: "Michael Paquier" 

On Fri, Jan 10, 2014 at 12:08 AM, MauMau  wrote:
C2. "recovery_target = 'immediate'" sounds less intuitive than my 
suggestion

"recovery_target_time = 'backup_point'", at least for those who want to
recover to the backup point.
Although I don't have a good naming sense in English, the value should be 
a

noun, not an adjective like "immediate", because the value specifies the
"target (point)" of recovery.

"immediate" is perfectly fine IMO, it fits with what this recovery
target aims at: an immediate consistency point. My 2c on that.


OK, I believe the naming sense of people whose mother tongue is English.  I 
thought the value should be a noun like "earliest_consistency_point" or 
"earliest_consistency" (I don't these are good, though).


Regards
MauMau



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

Re: [HACKERS] Standalone synchronous master

[Amit Kapila]

On Sun, Jan 12, 2014 at 8:48 AM, Josh Berkus  wrote:
> On 01/10/2014 06:27 PM, Bruce Momjian wrote:
>> How would that work?  Would it be a tool in contrib?  There already is a
>> timeout, so if a tool checked more frequently than the timeout, it
>> should work.  The durable notification of the admin would happen in the
>> tool, right?
>
> Well, you know what tool *I'm* planning to use.
>
> Thing is, when we talk about auto-degrade, we need to determine things
> like "Is the replica down or is this just a network blip"? and take
> action according to the user's desired configuration.  This is not
> something, realistically, that we can do on a single request.  Whereas
> it would be fairly simple for an external monitoring utility to do:
>
> 1. decide replica is offline for the duration (several poll attempts
> have failed)
>
> 2. Send ALTER SYSTEM SET to the master and change/disable the
> synch_replicas.

   Will it possible in current mechanism, because presently master will
   not accept any new command when the sync replica is not available?
   Or is there something else also which needs to be done along with
   above 2 points to make it possible.


With Regards,
Amit Kapila.
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] [GENERAL] pg_upgrade & tablespaces

[Bruce Momjian]

On Sat, Jan 11, 2014 at 12:48:51PM -0800, Adrian Klaver wrote:
> >pg_upgrade looks in the pg_tablespace in pre-9.2, and uses a function in
> >9.2+.  The query is:
> >
> > snprintf(query, sizeof(query),
> >  "SELECT%s "
> >  "FROM  pg_catalog.pg_tablespace "
> >  "WHERE spcname != 'pg_default' AND "
> >  "  spcname != 'pg_global'",
> > /* 9.2 removed the spclocation column */
> >  (GET_MAJOR_VERSION(old_cluster.major_version) <= 901) ?
> >--> "spclocation" : "pg_catalog.pg_tablespace_location(oid) AS spclocation");
> 
> 
> I see, though I have another question. If pg_tablespace and the
> symlinks can get out of sync, as you say below, why is pg_tablespace
> considered the authority? Or to put it another way, why not just
> look at the symlinks as in 9.2+?

Uh, good question.  I think I used the system tables because they were
easier to access.  I can't remember if we used the symlinks for some
things and pg_tablespace for other things in pre-9.2.

> >I think the big question on adding a check is, how many users of 9.4 are
> >going to be upgrading from pre-9.2 and have tablespaces in PGDATA, and
> >will be renaming their old PGDATA directory during the upgrade?  We
> >could add the test to 9.3 too, of course.
> 
> Well it is not generally accepted that users should even be creating
> tablespaces in $PGDATA, but it is allowed by the program. My
> inclination is to say that it is then the programs'(Postgres)
> responsibility to deal with it. The alternative is to clarify the
> documentation and make it the users responsibility. As to users
> upgrading from 9.1- to 9.2+, I see still a lot of users posting to
> --general using 9.1- versions. At some point they will likely
> migrate, so I can see a fix being worthwhile.

True.

> >Having pg_tablespace and the symlinks get out of sync was the reason
> >Magnus removed that duplication in 9.2, but I was unaware of how
> >pg_upgrade really magnifies the problem for tablespaces in PGDATA by
> >recommending a PGDATA rename.
> >
> 
> Well it was based on the valid assumption that people would create
> new tablespaces outside $PGDATA because that is really is what is
> meant to happen. You know us users we like to test assumptions:)

Also true.  :-)

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

  + Everyone has their own god. +


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

Re: [HACKERS] Standalone synchronous master

[Bruce Momjian]

On Sat, Jan 11, 2014 at 07:18:02PM -0800, Josh Berkus wrote:
> In other words, if we're going to have auto-degrade, the most
> intelligent place for it is in
> RepMgr/HandyRep/OmniPITR/pgPoolII/whatever.  It's also the *easiest*
> place.  Anything we do *inside* Postgres is going to have a really,
> really hard time determining when to degrade.

Well, one goal I was considering is that if a commit is hung waiting for
slave sync confirmation, and the timeout happens, then the mode is
changed to degraded and the commit returns success.  I am not sure how
you would do that in an external tool, meaning there is going to be
period where commits fail, unless you think there is a way that when the
external tool changes the mode to degrade that all hung commits
complete.  That would be nice.

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

  + Everyone has their own god. +


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

Re: [HACKERS] Standalone synchronous master

[Amit Kapila]

On Sat, Jan 11, 2014 at 9:41 PM, Bruce Momjian  wrote:
> On Sat, Jan 11, 2014 at 01:29:23PM +0530, Amit Kapila wrote:
>> Okay, this is one way of providing this new mode, others could be:
>>
>> a.
>> Have just one GUC sync_standalone_mode = true|false and make
>> this as PGC_POSTMASTER parameter, so that user is only
>> allowed to set this mode at startup. Even if we don't want it as
>> Postmaster parameter, we can mention to users that they can
>> change this parameter only before server reaches current situation.
>> I understand that without any alarm or some other way, it is difficult
>> for user to know and change it, but I think in that case he should
>> set it before server startup.
>>
>> b.
>> On above lines, instead of boolean parameter, provide a parameter
>> similar to current one such as available_synchronous_standby_names,
>> setting of this should follow what I said in point a. The benefit in this
>> as compare to 'a' is that it appears to be more like what we currently have.
>>
>> I think if we try to solve this problem by providing a way so that user
>> can change it at runtime or when the problem actually occurred, it can
>> make the UI more complex and difficult for us to provide a way so that
>> user can be alerted on such situation. We can keep our options open
>> so that if tomorrow, we can find any reasonable way, then we can
>> provide it to user a mechanism for changing this at runtime, but I don't
>> think it is stopping us from providing a way with which user can get the
>> benefit of this mode by providing start time parameter.
>
> I am not sure how this would work.  Right now we wait for one of the
> synchronous_standby_names servers to verify the writes.   We need some
> way of telling the system how long to wait before continuing in degraded
> mode.  Without a timeout and admin notification, it doesn't seem much
> better than our async mode, which is what many people were complaining
> about.

It is better than async mode in a way such that in async mode it never
waits for commits to be written to standby, but in this new mode it will
do so unless it is not possible (all sync standby's goes down).
Can't we use existing wal_sender_timeout, or even if user expects a
different timeout because for this new mode, he expects master to wait
more before it start operating like standalone sync master, we can provide
a new parameter.

With this the definition of new mode is to provide maximum
availability.

We can define the behavior in this new mode as:
a. It will operate like current synchronous master till one of the standby
mentioned in available_synchronous_standby_names is available.
b. If none is available, then it will start operating link current async
master, which means that if any async standby is configured, then
it will start sending WAL to that standby asynchronously, else if none
is configured, it will start operating in a standalone master.
c. We can even provide a new parameter replication_mode here
(non persistent), which will tell to user that master has switched
its mode, this can be made available by view. Update the value of
parameter when server switches to new mode.
d. When one of the standby mentioned in
available_synchronous_standby_names comes back and able to resolve
all WAL difference, then it will again switch back to sync mode, where it
will write to that standby before Commit finishes. After switch, it will
update the replication_mode parameter.

Now I think with above definition and behavior, it can switch to new mode
and will be able to provide information if user wants it by using view.

In above behaviour, the tricky part would be point 'd' where it has to switch
back to sync mode when one of the sync standby become available, but I
think we can workout design for that if you are positive about the above
definition and behaviour as defined by 4 points.


With Regards,
Amit Kapila.
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] array_length(anyarray)

[Marko Tiikkaja]

On 1/9/14, 2:57 PM, Dean Rasheed wrote:

Yes, this should just return the number of elements, and 0 for an empty array.

How it should behave for multi-dimensional arrays is less clear, but
I'd argue that it should return the total number of elements, i.e.
cardinality('{{1,2},{3,4}}'::int[][]) = 4. That would make it
consistent with the choices we've already made for unnest() and
ordinality:
  - cardinality(foo) = (select count(*) from unnest(foo)).
  - unnest with ordinality would always result in ordinals in the range
[1, cardinality].


Ignoring my proposal, this seems like the most reasonable option.  I'll 
send an updated patch along these lines.



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

[HACKERS] have to do move ready for commit patch from previous commitfest?

[Pavel Stehule]

Hello

There are a few patches, that are ready for commit:

https://commitfest.postgresql.org/action/commitfest_view?id=20&status=3

their registration is valid still?

Regards

Pavel

Re: [HACKERS] array_length(anyarray)

[Pavel Stehule]

2014/1/12 Marko Tiikkaja 

> On 1/9/14, 2:57 PM, Dean Rasheed wrote:
>
>> Yes, this should just return the number of elements, and 0 for an empty
>> array.
>>
>> How it should behave for multi-dimensional arrays is less clear, but
>> I'd argue that it should return the total number of elements, i.e.
>> cardinality('{{1,2},{3,4}}'::int[][]) = 4. That would make it
>> consistent with the choices we've already made for unnest() and
>> ordinality:
>>   - cardinality(foo) = (select count(*) from unnest(foo)).
>>   - unnest with ordinality would always result in ordinals in the range
>> [1, cardinality].
>>
>
> Ignoring my proposal, this seems like the most reasonable option.  I'll
> send an updated patch along these lines.
>

+1

Pavel


>
>
> Regards,
> Marko Tiikkaja
>

[HACKERS] plpgsql.consistent_into

[Marko Tiikkaja]

Greetings fellow elephants,

I would humbly like to submit for your consideration my proposal for 
alleviating pain caused by one of the most annoying footguns in 
PL/PgSQL: the behaviour of SELECT .. INTO when the query returns more 
than one row.  Some of you might know that no exception is raised in 
this case (as opposed to INSERT/UPDATE/DELETE .. INTO, all of them 
yielding TOO_MANY_ROWS), which can hide subtle bugs in queries if during 
testing the query always returns only one row or the "correct" one 
happens to be picked up every time.  Additionally, the row_count() after 
execution is always going to be either 0 or 1, so even if you want to 
explicitly guard against potentially broken queries, you can't do so!


So I added the following compile-time option:


set plpgsql.consistent_into to true;

create or replace function footest() returns void as $$
declare
x int;
begin
  -- too many rows
  select 1 from foo into x;
end$$ language plpgsql;

select footest();
ERROR:  query returned more than one row

It defaults to false to preserve full backwards compatibility.  Also 
turning it on makes the executor try and find two rows, so it might have 
an effect on performance as well.  The patch, as currently written, also 
changes the behaviour of EXECUTE .. INTO, but I don't feel strongly 
about whether that should be affected as well or not.



Regards,
Marko Tiikkaja
*** a/doc/src/sgml/plpgsql.sgml
--- b/doc/src/sgml/plpgsql.sgml
***
*** 1043,1050  DELETE ... RETURNING expressions 
INTO STRIC
   clause, then target will be set to the first
   row returned by the query, or to nulls if the query returned no rows.
   (Note that the first row is not
!  well-defined unless you've used ORDER BY.)  Any result rows
!  after the first row are discarded.
   You can check the special FOUND variable (see
   ) to
   determine whether a row was returned:
--- 1043,1053 
   clause, then target will be set to the first
   row returned by the query, or to nulls if the query returned no rows.
   (Note that the first row is not
!  well-defined unless you've used ORDER BY.)  By default,
!  any result rows after the first row are discarded.  However, if the
!  consistent_into compile option has been enabled and the
!  query returns more than one row, the run-time error
!  TOO_MANY_ROWS will be reported.
   You can check the special FOUND variable (see
   ) to
   determine whether a row was returned:
***
*** 1172,1178  EXECUTE command-string  INT
   record variable is used, it will configure itself to match the
   result structure automatically). If multiple rows are returned,
   only the first will be assigned to the INTO
!  variable. If no rows are returned, NULL is assigned to the
   INTO variable(s). If no INTO
   clause is specified, the query results are discarded.
  
--- 1175,1183 
   record variable is used, it will configure itself to match the
   result structure automatically). If multiple rows are returned,
   only the first will be assigned to the INTO
!  variable, unless consistent_into has been
!  enabled, in which case TOO_MANY_ROWS is raised.
!  If no rows are returned, NULL is assigned to the
   INTO variable(s). If no INTO
   clause is specified, the query results are discarded.
  
*** a/src/pl/plpgsql/src/pl_comp.c
--- b/src/pl/plpgsql/src/pl_comp.c
***
*** 352,357  do_compile(FunctionCallInfo fcinfo,
--- 352,358 
function->out_param_varno = -1; /* set up for no OUT param */
function->resolve_option = plpgsql_variable_conflict;
function->print_strict_params = plpgsql_print_strict_params;
+   function->consistent_into = plpgsql_consistent_into;
  
if (is_dml_trigger)
function->fn_is_trigger = PLPGSQL_DML_TRIGGER;
***
*** 849,854  plpgsql_compile_inline(char *proc_source)
--- 850,856 
function->out_param_varno = -1; /* set up for no OUT param */
function->resolve_option = plpgsql_variable_conflict;
function->print_strict_params = plpgsql_print_strict_params;
+   function->consistent_into = plpgsql_consistent_into;
  
plpgsql_ns_init();
plpgsql_ns_push(func_name);
*** a/src/pl/plpgsql/src/pl_exec.c
--- b/src/pl/plpgsql/src/pl_exec.c
***
*** 3308,3316  exec_stmt_execsql(PLpgSQL_execstate *estate,
  
/*
 * If we have INTO, then we only need one row back ... but if we have 
INTO
!* STRICT, ask for two rows, so that we can verify the statement returns
!* only one.  INSERT/UPDATE/DELETE are always treated strictly. Without
!* INTO, just run the statement to completion (tcount = 0).
 *
 * We could just ask for two rows always when using INTO, but there are
 * some cases where demanding the extra row costs si

Re: [HACKERS] plpgsql.consistent_into

[Pavel Stehule]

Hello


2014/1/12 Marko Tiikkaja 

> Greetings fellow elephants,
>
> I would humbly like to submit for your consideration my proposal for
> alleviating pain caused by one of the most annoying footguns in PL/PgSQL:
> the behaviour of SELECT .. INTO when the query returns more than one row.
>  Some of you might know that no exception is raised in this case (as
> opposed to INSERT/UPDATE/DELETE .. INTO, all of them yielding
> TOO_MANY_ROWS), which can hide subtle bugs in queries if during testing the
> query always returns only one row or the "correct" one happens to be picked
> up every time.  Additionally, the row_count() after execution is always
> going to be either 0 or 1, so even if you want to explicitly guard against
> potentially broken queries, you can't do so!
>

It is not bad and, sure, - it is very useful and important

but - it is a redundant to INTO STRICT clause. When you use it, then you
change a INTO behaviour. Is not better to ensure STRICT option than hidden
redefining INTO?

Option INTO (without STRICT clause) is not safe and we should to disallow.
I see a three states (not only two)

a) disallow INTO without STRICT (as preferred for new code)
b) implicit check after every INTO without STRICT
c) without check

these modes should be: "strict_required", "strict_default", "strict_legacy"



> So I added the following compile-time option:
>
>
> set plpgsql.consistent_into to true;
>

This name is not best (there is not clean with it a into should be
consistent)

Is question, if this functionality should be enabled by GUC to be used for
legacy code (as protection against some kind of hidden bugs)

This topic is interesting idea for me - some checks can be pushed to
plpgsql_check (as errors or warnings) too.

Generally I like proposed functionality, just I am not sure, so hidden
redefining INTO clause (to INTO STRICT) is what we want. We can do it (but
explicitly). I don't know any situation where INTO without STRICT is valid.
Introduction of STRICT option was wrong idea - and now is not way to back.

Regards

Pavel



>
> create or replace function footest() returns void as $$
> declare
> x int;
> begin
>   -- too many rows
>   select 1 from foo into x;
> end$$ language plpgsql;
>
> select footest();
> ERROR:  query returned more than one row
>
> It defaults to false to preserve full backwards compatibility.  Also
> turning it on makes the executor try and find two rows, so it might have an
> effect on performance as well.  The patch, as currently written, also
> changes the behaviour of EXECUTE .. INTO, but I don't feel strongly about
> whether that should be affected as well or not.
>
>
> 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
>
>

[HACKERS] Retain dynamic shared memory segments for postmaster lifetime

[Amit Kapila]

Currently there is no way user can keep the dsm
segments if he wants for postmaster lifetime, so I
have exposed a new API dsm_keep_segment()
to implement the same.

The specs and need for this API is already discussed
in thread:
http://www.postgresql.org/message-id/ca+tgmoakogujqbedgeykysxud9eaidqx77j2_hxzrgfo3hr...@mail.gmail.com

I had used dsm_demo (hacked it a bit) module used
during initial tests for dsm API's to verify the working on
Windows. So one idea could be that I can extend
that module to use this new API, so that it can be tested
by others as well or if you have any other better way, please
do let me know.


As the discussion about its specs and need is already
discussed in above mentioned thread, so I will upload
this patch to CF unless there is any Objection.


With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


dsm_keep_segment_v1.patch
Description: Binary data

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

Re: [HACKERS] PostgreSQL Service on Windows does not start if data directory given is relative path

[David Rowley]

On Tue, Jan 7, 2014 at 12:42 AM, Rajeev rastogi
wrote:

>  I have found a case that PostgreSQL as win32 service does not start, if
> the data directory given as relative path.
>
> Error observed in this case is:
>
> “The PostgreSQL on Local Computer started
> and then stopped”.
>
>
>
> This may happen because relative path given will be relative to path from
> where service is registered but
>
> the path from where WIN32 service execution gets invoked may be different
> and hence it won’t be able
>
> to find the  data directory.
>
>
>
> I have fixed the same by internally converting the relative path to
> absolute path as it is being done for executable file.
>
>
>
> Attached is the patch with the fix.
>
> Please provide your opinion.
>
>
Hi,

I've not quite got around to testing this yet, but I think the patch is a
good idea as I can see that I can use a relative path when I start
postgres.exe from the command line, I guess the behaviour should likely be
the same when installed as a windows service.

So, I've just been looking over this patch and I'm just wondering about a
few things:

In pgwin32_CommandLine you declare dbPath, it looks like you could declare
this in the scope of; if (pg_config)

In find_my_abs_path you're making a call to StrNCpy, I know you likely just
used find_my_exec as an example here, but I'd say the use of StrNCpy is not
really needed here and is a bit of a waste of cycles. I'd rather see
strlcpy being used as strncpy will needlessly zero the remaining buffer
space.

Also in find_my_abs_path, I'm just wondering if the cwd variable is really
needed, I think you could just use retpath each time and it would also save
a little bit of copying that's done in join_path_components(). By the looks
of it you can just call join_path_components(retpath, retpath, inpath).
Perhaps some people would disagree, but I'm not really seeing the harm in
it and it saves some copying too.

Regards

David Rowley



>
>
> I will add this to Jan 2014 CommitFest.
>
>
>
> *Thanks and Regards,*
>
> *Kumar Rajeev Rastogi *
>
>
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
>

[HACKERS] Why does numeric_out produce so many trailing zeros?

[David Rowley]

I've been reading the documents on numeric and I can't find any information
on the reason that a query like this:

test=# select n::numeric / 1 from generate_series(1,2) s(n);
?column?

 1.
 2.
(2 rows)

produces results that have so many trailing zeros. Also I'm wondering why
the first row has 20 trailing zeros and the 2nd row has just 16?

Is there any reason that we output any trailing zeros at all?

Regards

David Rowley