Re: [HACKERS] Clock with Adaptive Replacement

[Andrey Borodin]

Hi!

> 30 апр. 2018 г., в 23:15, Andres Freund  написал(а):
> On 2018-04-30 15:39:08 +0500, Andrey Borodin wrote:
>> I suspect that performance benefits can be not that big or even
>> marginal, if we do not extend comprehensive eviction strategy with
>> bgwriter fixes and O_DIRECT.
> 
> If so, then the improvements aren't real. Bgwriter doesn't matter for
> read-only workloads. O_DIRECT doesn't matter much if shared_buffers are
> 60+% of OS memory.  And even disregarding that, you can just compute
> cache hit ratios to see whether things are improving.
Even considering simply changing eviction strategy - it is not just about hit 
ratio. It is also about eviction complexity less than O(N).

But I think you are right. If we compare performance effect of half-measures in 
the real system, probably it is more accurate than comparing isolated 
algorithms in a sand box.

Best regards, Andrey Borodin.

Re: "could not reattach to shared memory" on buildfarm member dory

[Tom Lane]

Andres Freund  writes:
> Heath, could you use process explorer or such to check which processes
> are running inside a working backend process?

It seems to be possible to enumerate the threads that are present inside a
Windows process, although it's not clear to me how much identifying info
is available.  Perhaps it'd be worth putting in some "dump threads"
debugging code like the "dump modules" code we had in there for a bit?

regards, tom lane

Re: "could not reattach to shared memory" on buildfarm member dory

[Thomas Munro]

On Tue, May 1, 2018 at 2:59 PM, Noah Misch  wrote:
> Likely some privileged daemon is creating a thread in every new process.  (On
> Windows, it's not unusual for one process to create a thread in another
> process.)  We don't have good control over that.

Huh.  I was already amazed (as a non-Windows user) by the DSM code
that duplicates file handles into the postmaster process without its
cooperation, but starting threads is even more amazing.
Apparently debuggers do that.  Could this be running in some kind of
debugger-managed environment or build, perhaps as a result of some
core dump capturing mode or something?

https://msdn.microsoft.com/en-us/library/windows/desktop/dd405484(v=vs.85).aspx

Apparently another way to mess with another process's memory map is
via "Asynchronous Procedure Calls":

http://blogs.microsoft.co.il/pavely/2017/03/14/injecting-a-dll-without-a-remote-thread/

It looks like that mechanism could allow something either in our own
process (perhaps some timer-related thing that we might have set up
ourselves or might be set up by the system?) or another process to
queue actions for our own thread to run at certain points.

https://msdn.microsoft.com/en-us/library/windows/desktop/ms681951(v=vs.85).aspx

-- 
Thomas Munro
http://www.enterprisedb.com

Re: [HACKERS] proposal: schema variables

[Pavel Stehule]

Hi

2018-05-01 3:56 GMT+02:00 Peter Eisentraut :

> On 4/20/18 13:45, Pavel Stehule wrote:
> > I dunno, it seems awfully different to me.  There's only one
> "column",
> > right?  What code is really shared here?  Are constraints and
> triggers
> > even desirable feature for variables?  What would be the use case?
> >
> >
> > The schema variable can hold composite value. The patch allows to use
> > any composite type or adhoc composite values
> >
> > DECLARE x AS compositetype;
> > DECLARE x AS (a int, b int, c int);
>
> I'm not sure that this anonymous composite type thing is such a good
> idea.  Such a variable will then be incompatible with anything else,
> because it's of a different type.
>

Using anonymous composite type variable is just shortcut for situations
when mentioned feature is not a problem. These variables are global, so
there can be only one variable of some specific composite type, and
incompatibility with others is not a issue.

This feature can be interesting for short live temp variables - these
variables can be used for parametrization of anonymous block.

But this feature is not significant, and can be removed from patch.


> In any case, I find that a weak argument for storing this in pg_class.
> You could just as well create these pg_class entries implicitly and link
> them from "pg_variable", same as composite types have a main entry in
> pg_type and additional stuff in pg_class.
>
> > I think stuffing this into pg_class is pretty strange.
> >
> > It will be if variable is just scalar value without any possibilities.
> > But then there is only low benefit
> >
> > The access rights implementation is shared with other from pg_class too.
>
> In DB2, the privileges for variables are named READ and WRITE.  That
> would make more sense to me than reusing the privilege names for tables.
>
>
good idea

Regards

Pavel

> --
> Peter Eisentraut  http://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>

Re: "could not reattach to shared memory" on buildfarm member dory

[Noah Misch]

On Mon, Apr 30, 2018 at 08:01:40PM -0400, Tom Lane wrote:
> It's clear from dory's results that something is causing a 4MB chunk
> of memory to get reserved in the process's address space, sometimes.
> It might happen during the main MapViewOfFileEx call, or during the
> preceding VirtualFree, or with my map/unmap dance in place, it might
> happen during that.  Frequently it doesn't happen at all, at least not
> before the point where we've successfully done MapViewOfFileEx.  But
> if it does happen, and the chunk happens to get put in a spot that
> overlaps where we want to put the shmem block, kaboom.
> 
> What seems like a plausible theory at this point is that the apparent
> asynchronicity is due to the allocation being triggered by a different
> thread, and the fact that our added monitoring code seems to make the
> failure more likely can be explained by that code changing the timing.
> But what thread could it be?  It doesn't really look to me like either
> the signal thread or the timer thread could eat 4MB.  syslogger.c
> also spawns a thread, on Windows, but AFAICS that's not being used in
> this test configuration.  Maybe the reason dory is showing the problem
> is something or other is spawning a thread we don't even know about?

Likely some privileged daemon is creating a thread in every new process.  (On
Windows, it's not unusual for one process to create a thread in another
process.)  We don't have good control over that.

> I'm at a loss for a reasonable way to fix it
> for real.  Is there a way to seize control of a Windows process so that
> there are no other running threads?

I think not.

> Any other ideas?

PostgreSQL could retry the whole process creation, analogous to
internal_forkexec() retries.  Have the failed process exit after recording the
fact that it couldn't attach.  Make the postmaster notice and spawn a
replacement.  Give up after 100 failed attempts.

RE: power() function in Windows: "value out of range: underflow"

[Huong Dangminh]

> From: Tom Lane [mailto:t...@sss.pgh.pa.us]
> > I updated the patch as David Rowley mentioned.
> 
> Pushed.  I'd mainly note that you need to update all the variant float8
> expected-files, not just the primary one.  (Sometimes this requires a bit

Thank you.
I will be careful next time.

> of guesswork, but here we're expecting all platforms to produce the same
> result.  The buildfarm should tell us if I got it wrong.)

Also thanks a lot of fixing failure in BSD platform as the result from 
buildfarm.


---
Thanks and best regards,
Dang Minh Huong
NEC Solution Innovators, Ltd.
http://www.nec-solutioninnovators.co.jp/en/

Re: [HACKERS] proposal: schema variables

[Peter Eisentraut]

On 4/20/18 13:45, Pavel Stehule wrote:
> I dunno, it seems awfully different to me.  There's only one "column",
> right?  What code is really shared here?  Are constraints and triggers
> even desirable feature for variables?  What would be the use case?
> 
> 
> The schema variable can hold composite value. The patch allows to use
> any composite type or adhoc composite values
> 
> DECLARE x AS compositetype;
> DECLARE x AS (a int, b int, c int);

I'm not sure that this anonymous composite type thing is such a good
idea.  Such a variable will then be incompatible with anything else,
because it's of a different type.

In any case, I find that a weak argument for storing this in pg_class.
You could just as well create these pg_class entries implicitly and link
them from "pg_variable", same as composite types have a main entry in
pg_type and additional stuff in pg_class.

> I think stuffing this into pg_class is pretty strange.
> 
> It will be if variable is just scalar value without any possibilities.
> But then there is only low benefit
> 
> The access rights implementation is shared with other from pg_class too.

In DB2, the privileges for variables are named READ and WRITE.  That
would make more sense to me than reusing the privilege names for tables.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: FOR EACH ROW triggers on partitioned tables

[Amit Langote]

On 2018/04/30 18:38, Ashutosh Bapat wrote:
> On Fri, Mar 23, 2018 at 7:19 PM, Alvaro Herrera  
> wrote:
>> Pushed.  Thanks for all the review.
>>
> 
> https://www.postgresql.org/docs/devel/static/ddl-partitioning.html
> 
> section 5.10.2.3 still says
> "Row triggers, if necessary, must be defined on individual partitions,
> not the partitioned table."
> 
> Should that change?
> 
> Per commit 86f575948c773b0ec5b0f27066e37dd93a7f0a96, we are supporting
> only AFTER row triggers. May be we should change the above line to
> "Before row triggers, if necessary, must ".

A patch to fix that has been posted.

https://www.postgresql.org/message-id/9386c128-1131-d115-cda5-63ac88d15db1%40lab.ntt.co.jp

Thanks,
Amit

Re: Postgres, fsync, and OSs (specifically linux)

[Craig Ringer]

On 1 May 2018 at 00:09, Andres Freund  wrote:

> It's not. Only SYNC_FILE_RANGE_WAIT_{BEFORE,AFTER} eat errors. Which
> seems sensible, because they could be considered data integrity
> operations.

Ah, I misread that. Thankyou.

>> I'm very suspicious about the safety of the msync() path too.
>
> That seems justified however:

I'll add EIO tests there.

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

Re: [HACKERS] Clock with Adaptive Replacement

[Юрий Соколов]

вт, 1 мая 2018 г., 0:02 Thomas Munro :

> On Mon, Apr 30, 2018 at 10:39 PM, Andrey Borodin 
> wrote:
> >> 30 апр. 2018 г., в 0:48, Andres Freund  написал(а):
> >> On 2018-04-25 11:31:12 +0500, Andrey Borodin wrote:
> >> 30 апр. 2018 г., в 0:39, Юрий Соколов 
> написал(а):
> >>
> >> (btw, why no one mention CLOCK+ in this thread).
> >
> > I do not know what CLOCK+ is. Can you, please, send me a reference.
>
> Maybe he means CLOCK-Pro?
>

Oh, thank you, Thomas.
Yes, I was telling about CLOCK-Pro.

Re: "could not reattach to shared memory" on buildfarm member dory

[Andres Freund]

Him

On 2018-04-30 20:01:40 -0400, Tom Lane wrote:
> What seems like a plausible theory at this point is that the apparent
> asynchronicity is due to the allocation being triggered by a different
> thread, and the fact that our added monitoring code seems to make the
> failure more likely can be explained by that code changing the timing.
> But what thread could it be?  It doesn't really look to me like either
> the signal thread or the timer thread could eat 4MB.

It seems plausible that the underlying allocator allocates larger chunks
to serve small allocations.  But we don't seem to have started any threads
at PGSharedMemoryReAttach() time? So it'd have to be something else that
starts threads.

Heath, could you use process explorer or such to check which processes
are running inside a working backend process?

Greetings,

Andres Freund

Re: [HACKERS] Clock with Adaptive Replacement

[Andres Freund]

On 2018-05-01 12:15:21 +1200, Thomas Munro wrote:
> On Thu, Apr 26, 2018 at 1:31 PM, Thomas Munro
>  wrote:
> > ...  I
> > suppose when you read a page in, you could tell the kernel that you
> > POSIX_FADV_DONTNEED it, and when you steal a clean PG buffer you could
> > tell the kernel that you POSIX_FADV_WILLNEED its former contents (in
> > advance somehow), on the theory that the coldest stuff in the PG cache
> > should now become the hottest stuff in the OS cache.  Of course that
> > sucks, because the best the kernel can do then is go and read it from
> > disk, and the goal is to avoid IO.  Given a hypothetical way to
> > "write" "clean" data to the kernel (so it wouldn't mark it dirty and
> > generate IO, but it would let you read it back without generating IO
> > if you're lucky), then perhaps you could actually achieve exclusive
> > caching at the two levels, and use all your physical RAM without
> > duplication.
> 
> Craig said essentially the same thing, on the nearby fsync() reliability 
> thread:
> 
> On Sun, Apr 29, 2018 at 1:50 PM, Craig Ringer  wrote:
> > ... I'd kind of hoped to go in
> > the other direction if anything, with some kind of pseudo-write op
> > that let us swap a dirty shared_buffers entry from our shared_buffers
> > into the OS dirty buffer cache (on Linux at least) and let it handle
> > writeback, so we reduce double-buffering. Ha! So much for that!
> 
> I would like to reply to that on this thread which discusses double
> buffering and performance, to avoid distracting the fsync() thread
> from its main topic of reliability.

It's not going to happen. Robert and I talked to the kernel devs a
couple years back, and I've brought it up again. The kernel has
absolutely no chance to verify the content of that written data, meaning
that suddenly you'd get differing data based on cache pressure. It seems
unsurprising that kernel devs aren't wild about that idea. The
likelihood of that opening up weird exploits (imagine a suid binary
reading such data later!), seems also pretty high.

Greetings,

Andres Freund

Re: [HACKERS] Clock with Adaptive Replacement

[Thomas Munro]

On Thu, Apr 26, 2018 at 1:31 PM, Thomas Munro
 wrote:
> ...  I
> suppose when you read a page in, you could tell the kernel that you
> POSIX_FADV_DONTNEED it, and when you steal a clean PG buffer you could
> tell the kernel that you POSIX_FADV_WILLNEED its former contents (in
> advance somehow), on the theory that the coldest stuff in the PG cache
> should now become the hottest stuff in the OS cache.  Of course that
> sucks, because the best the kernel can do then is go and read it from
> disk, and the goal is to avoid IO.  Given a hypothetical way to
> "write" "clean" data to the kernel (so it wouldn't mark it dirty and
> generate IO, but it would let you read it back without generating IO
> if you're lucky), then perhaps you could actually achieve exclusive
> caching at the two levels, and use all your physical RAM without
> duplication.

Craig said essentially the same thing, on the nearby fsync() reliability thread:

On Sun, Apr 29, 2018 at 1:50 PM, Craig Ringer  wrote:
> ... I'd kind of hoped to go in
> the other direction if anything, with some kind of pseudo-write op
> that let us swap a dirty shared_buffers entry from our shared_buffers
> into the OS dirty buffer cache (on Linux at least) and let it handle
> writeback, so we reduce double-buffering. Ha! So much for that!

I would like to reply to that on this thread which discusses double
buffering and performance, to avoid distracting the fsync() thread
from its main topic of reliability.

I think that idea has potential.  Even though I believe that direct IO
is the generally the right way to go (that's been RDBMS orthodoxy for
a decade or more AFAIK), we'll always want to support buffered IO (as
other RDBMSs do).  For one thing, not every filesystem supports direct
IO, including ZFS.  I love ZFS, and its caching is not simply a dumb
extension to shared_buffers that you have to go through syscalls to
reach: it has state of the art page reclamation, cached data can be
LZ4 compressed and there is an optional second level cache which can
live on fast storage.

Perhaps if you patched PostgreSQL to tell the OS that you won't need
pages you've just read, and that you will need pages you've just
evicted, you might be able to straighten out some of that U shape by
getting more exclusive caching at the two levels.  Queued writes would
still be double-buffered of course, at least until they complete.
Telling the OS to prefetch something that you already have a copy of
is annoying and expensive, though.

The pie-in-the-sky version of this idea would let you "swap" pages
with the kernel, as you put it.  Though I was thinking of clean pages,
not dirty ones.  Then there'd be a non-overlapping set of  pages from
your select-only pgbench in each cache.  Maybe that would look like
punread(fd, buf, size, offset) (!), or maybe write(fd, buf, size)
followed by fadvise(fd, offset, size,
FADV_I_PERSONALLY_GUARANTEE_THIS_DATA_IS_CLEAN_AND_I_CONSIDERED_CONCURRENCY_VERY_CAREFULLY),
or maybe pswap(read params... , unread params ...) to read new buffer
and unread old buffer at the same time.

Sadly, even if the simple non-pie-in-the-sky version of the above were
to work out and be beneficial on your favourite non-COW filesystem (on
which you might as well use direct IO and larger shared_buffers, some
day), it may currently be futile on ZFS because I think the fadvise
machinery might not even be hooked up (Solaris didn't believe in
fadvise on any filesystem IIRC).  Not sure, I hope I'm wrong about
that.

-- 
Thomas Munro
http://www.enterprisedb.com

Re: "could not reattach to shared memory" on buildfarm member dory

[Tom Lane]

I wrote:
> The solution I was thinking about last night was to have
> PGSharedMemoryReAttach call MapViewOfFileEx to map the shared memory
> segment at an unspecified address, then unmap it, then call VirtualFree,
> and finally call MapViewOfFileEx with the real target address.  The idea
> here is to get these various DLLs to set up any memory allocation pools
> they're going to set up before we risk doing VirtualFree.  I am not,
> at this point, convinced this will fix it :-( ... but I'm not sure what
> else to try.

So the answer is that that doesn't help at all.

It's clear from dory's results that something is causing a 4MB chunk
of memory to get reserved in the process's address space, sometimes.
It might happen during the main MapViewOfFileEx call, or during the
preceding VirtualFree, or with my map/unmap dance in place, it might
happen during that.  Frequently it doesn't happen at all, at least not
before the point where we've successfully done MapViewOfFileEx.  But
if it does happen, and the chunk happens to get put in a spot that
overlaps where we want to put the shmem block, kaboom.

What seems like a plausible theory at this point is that the apparent
asynchronicity is due to the allocation being triggered by a different
thread, and the fact that our added monitoring code seems to make the
failure more likely can be explained by that code changing the timing.
But what thread could it be?  It doesn't really look to me like either
the signal thread or the timer thread could eat 4MB.  syslogger.c
also spawns a thread, on Windows, but AFAICS that's not being used in
this test configuration.  Maybe the reason dory is showing the problem
is something or other is spawning a thread we don't even know about?

I'm going to go put a 1-sec sleep into the beginning of
PGSharedMemoryReAttach and see if that changes anything.  If I'm right
that this is being triggered by another thread, that should allow the
other thread to do its thing (at least most of the time) so that the
failure rate ought to go way down.

Even if that does happen, I'm at a loss for a reasonable way to fix it
for real.  Is there a way to seize control of a Windows process so that
there are no other running threads?  Any other ideas?

regards, tom lane

Re: EXECUTE does not process parameters

[Tom Lane]

Peter Eisentraut  writes:
> The SQL-level EXECUTE statement does not process parameters in its
> arguments.  So for example, the following does not work:
> ...
> It seems this needs some special treatment in transformStmt().  Any
> reason not to do that?

I have a vague recollection of having poked at this years ago and
found out that there were some interesting corner cases to worry about.
I think they had to do with parameters being passed through multiple
execution layers, but I don't remember for sure.  All I can say is
that this is probably trickier than it looks.

regards, tom lane

Re: Fsync request queue

[Andres Freund]

On 2018-04-30 16:07:48 -0700, Peter Geoghegan wrote:
> On Mon, Apr 30, 2018 at 4:03 PM, Andres Freund  wrote:
> >> Is this a problem in practice, though? I don't remember seeing any reports
> >> of the fsync queue filling up, after we got the code to compact it. I don't
> >> know if anyone has been looking for that, so that might also explain the
> >> absence of reports, though.
> >
> > It's probably hard to diagnose that as the origin of slow IO from the
> > outside. It's not exactly easy to diagnose that even if you know what's
> > going on.
> 
> True, but has anyone ever actually observed a non-zero
> pg_stat_bgwriter.buffers_backend_fsync in the wild after the
> compaction queue stuff was added/backpatched?

Yes.

Greetings,

Andres Freund

Re: Fsync request queue

[Peter Geoghegan]

On Mon, Apr 30, 2018 at 4:03 PM, Andres Freund  wrote:
>> Is this a problem in practice, though? I don't remember seeing any reports
>> of the fsync queue filling up, after we got the code to compact it. I don't
>> know if anyone has been looking for that, so that might also explain the
>> absence of reports, though.
>
> It's probably hard to diagnose that as the origin of slow IO from the
> outside. It's not exactly easy to diagnose that even if you know what's
> going on.

True, but has anyone ever actually observed a non-zero
pg_stat_bgwriter.buffers_backend_fsync in the wild after the
compaction queue stuff was added/backpatched?

-- 
Peter Geoghegan

Re: Issues while building PG in MS Windows, using MSYS2 and MinGW-w64

[Yuriy Zhuravlev]

If you have time, can you check CMake version?
https://github.com/stalkerg/postgres_cmake

During Mingw installation you can choose a version, it can be more MSVC
like or more Unix, probably Postgres have a problem with one of mingw type.

 So I unpacked the source tarball within the shell itself(using gunzip and
> tar), and I could fix many errors.
>

This is really stange, mingw on windows should't depend on any attribute.

Re: Fsync request queue

[Andres Freund]

On 2018-04-25 09:19:52 +0300, Heikki Linnakangas wrote:
> On 24/04/18 21:00, Andres Freund wrote:
> > Right now if the queue is full and can't be compacted we end up
> > fsync()ing on every single write, rather than once per checkpoint
> > afaict. That's a fairly horrible.
> > 
> > For the case that there's no space in the map, I'd suggest to just do
> > 10% or so of the fsync in the poor sod of a process that finds no
> > space. That's surely better than constantly fsyncing on every single
> > write.
> 
> Clever idea. In principle, you could do that even with the current queue,
> without changing it to a hash table.

Right. I was thinking of this in the context of the fsync mess, which
seems to require us to keep FDs open across processes for reliable error
detection. Which then made me look at register_dirty_segment(). Which in
turn made me think that it's weird that we do all that work even if it's
likely that it's been done before...


> Is this a problem in practice, though? I don't remember seeing any reports
> of the fsync queue filling up, after we got the code to compact it. I don't
> know if anyone has been looking for that, so that might also explain the
> absence of reports, though.

It's probably hard to diagnose that as the origin of slow IO from the
outside. It's not exactly easy to diagnose that even if you know what's
going on.

Greetings,

Andres Freund

Re: documentation is now XML

[Bruce Momjian]

On Mon, Apr 30, 2018 at 03:20:44PM -0400, Peter Eisentraut wrote:
> On 4/27/18 11:03, Bruce Momjian wrote:
> > On Fri, Apr 27, 2018 at 11:00:36AM -0400, Peter Eisentraut wrote:
> >> On 4/23/18 05:54, Liudmila Mantrova wrote:
> >>> Reading this thread, I got an impression that everyone would benefit
> >>> from converting back branches to XML, but the main concern is lack of
> >>> resources to complete this task. Are there any other issues that affect
> >>> this decision? Looks like Aleksander Lakhin's offer to prepare patches
> >>> was missed somehow as the discussion sidetracked to other issues
> >>
> >> That proposal seemed to indicate not only converting the source code to
> >> XML but also the build system to XSL.  The latter is out of the
> >> question, I think.
> > 
> > Why is that?
> Because there would be a thousand lines of tooling changes to be
> backported and thousands of pages of documentation to be checked
> manually that it doesn't create a mess (times branches times platforms).

Oh, OK.

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

+ As you are, so once was I.  As I am, so you will be. +
+  Ancient Roman grave inscription +

Re: obsoleting plpython2u and defaulting plpythonu to plpython3u

[Yuriy Zhuravlev]

And Gentoo too.

> PEP 394 points out that some distros (at least Arch) have already switched
>

Re: Support Python 3 tests under MSVC

[Yuriy Zhuravlev]

Interesting, this working fine more than year with CMake build version.

On Tue, 1 May 2018, 04:53 Andrew Dunstan, 
wrote:

>
> Here are a couple of patches, one for master and one for the back
> branches from 9.5 to 10 to allow testing of plpython3 and associated
> contrib modules when building with MSVC.
>
> I've tested this out on a Windows 2016 machine with Visual Studio 2017.
>
> I'd like to apply these - they only affect the testing script
> vcregress.pl, so they should be very low risk.
>
>
> cheers
>
> andrew
>
>
>
> --
> Andrew Dunstanhttps://www.2ndQuadrant.com
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>
>

EXECUTE does not process parameters

[Peter Eisentraut]

The SQL-level EXECUTE statement does not process parameters in its
arguments.  So for example, the following does not work:

create table t1 (a int);

prepare p1 as insert into t1 (a) values ($1);

create function f2(x int) returns int
language sql
as $$
execute p1($1);  -- HERE
select null::int;
$$;

select f2(2);
ERROR:  there is no parameter $1

Another variant of this problem that is perhaps more interesting in
practice is that you can't pass bind parameters to an EXECUTE call via
PQexecParams().

It seems this needs some special treatment in transformStmt().  Any
reason not to do that?

There is an adjacent question why EXECUTE is not allowed as the final
data-returning statement in an SQL-language function.  So I suspect the
answer to all this is that no one has ever seriously tried this before.

Come to think of it, it would probably also be useful if PREPARE did
parameter processing, again in order to allow use with PQexecParams().
There might be other statements currently omitted as well.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: Is there a memory leak in commit 8561e48?

[Peter Eisentraut]

On 4/27/18 02:44, Andrew Gierth wrote:
>> "Tom" == Tom Lane  writes:
> 
>  Tom> I would bet money that that "_SPI_current->internal_xact" thing is
>  Tom> wrong/inadequate.
> 
> In particular this looks wrong to me: after doing:
> 
> do $$
>   begin
> execute 'declare foo cursor with hold for select 1/x as a from (values 
> (1),(0)) v(x)';
> commit;  -- errors within the commit
>   end;
> $$;
> ERROR:  division by zero
> CONTEXT:  PL/pgSQL function inline_code_block line 1 at COMMIT
> 
> the SPI stack is not cleaned up at all, and _SPI_connected is >= 0 even
> when back at the main backend command loop.

The memory leak can be fixed by adding a pfree().

The example you show can be fixed by doing SPI cleanup in both
transaction abort and exception return to main loop, similar to other
cases that now have to handle these separately.  Patch attached.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 0315e6dcc583fe62e1c9b718b451ebbdf04a724e Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Mon, 30 Apr 2018 16:45:17 -0400
Subject: [PATCH] Fix SPI error cleanup and memory leak

Since the SPI stack has been moved from TopTransactionContext to
TopMemoryContext, the cleanup in AtEOXact_SPI() needs to run pfree() on
top of setting _SPI_stack to NULL.

Also, refactor the SPI cleanup so that it is run both at transaction end
and when returning to the main loop on an exception.  The latter is
necessary when a procedure calls a COMMIT or ROLLBACK command that
itself causes an error.
---
 src/backend/executor/spi.c  | 34 --
 src/backend/tcop/postgres.c |  2 ++
 src/include/executor/spi.h  |  1 +
 3 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index 08f6f67a15..31554cccf9 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -260,35 +260,41 @@ SPI_rollback(void)
_SPI_current->internal_xact = false;
 }
 
+/*
+ * Clean up SPI state.  Called on transaction end (of non-SPI-internal
+ * transactions) and when returning to the main loop on error.
+ */
+void
+SPICleanup(void)
+{
+   if (_SPI_stack)
+   pfree(_SPI_stack);
+   _SPI_stack = NULL;
+   _SPI_stack_depth = 0;
+   _SPI_current = NULL;
+   _SPI_connected = -1;
+   SPI_processed = 0;
+   SPI_lastoid = InvalidOid;
+   SPI_tuptable = NULL;
+}
+
 /*
  * Clean up SPI state at transaction commit or abort.
  */
 void
 AtEOXact_SPI(bool isCommit)
 {
-   /*
-* Do nothing if the transaction end was initiated by SPI.
-*/
+   /* Do nothing if the transaction end was initiated by SPI. */
if (_SPI_current && _SPI_current->internal_xact)
return;
 
-   /*
-* Note that memory contexts belonging to SPI stack entries will be 
freed
-* automatically, so we can ignore them here.  We just need to restore 
our
-* static variables to initial state.
-*/
if (isCommit && _SPI_connected != -1)
ereport(WARNING,
(errcode(ERRCODE_WARNING),
 errmsg("transaction left non-empty SPI stack"),
 errhint("Check for missing \"SPI_finish\" 
calls.")));
 
-   _SPI_current = _SPI_stack = NULL;
-   _SPI_stack_depth = 0;
-   _SPI_connected = -1;
-   SPI_processed = 0;
-   SPI_lastoid = InvalidOid;
-   SPI_tuptable = NULL;
+   SPICleanup();
 }
 
 /*
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 5095a4f686..4e0c8f074e 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -42,6 +42,7 @@
 #include "catalog/pg_type.h"
 #include "commands/async.h"
 #include "commands/prepare.h"
+#include "executor/spi.h"
 #include "jit/jit.h"
 #include "libpq/libpq.h"
 #include "libpq/pqformat.h"
@@ -3938,6 +3939,7 @@ PostgresMain(int argc, char *argv[],
WalSndErrorCleanup();
 
PortalErrorCleanup();
+   SPICleanup();
 
/*
 * We can't release replication slots inside AbortTransaction() 
as we
diff --git a/src/include/executor/spi.h b/src/include/executor/spi.h
index e5bdaecc4e..143a89a16c 100644
--- a/src/include/executor/spi.h
+++ b/src/include/executor/spi.h
@@ -163,6 +163,7 @@ extern void SPI_start_transaction(void);
 extern void SPI_commit(void);
 extern void SPI_rollback(void);
 
+extern void SPICleanup(void);
 extern void AtEOXact_SPI(bool isCommit);
 extern void AtEOSubXact_SPI(bool isCommit, SubTransactionId mySubid);
 
-- 
2.17.0

Re: [HACKERS] Clock with Adaptive Replacement

[Thomas Munro]

On Mon, Apr 30, 2018 at 10:39 PM, Andrey Borodin  wrote:
>> 30 апр. 2018 г., в 0:48, Andres Freund  написал(а):
>> On 2018-04-25 11:31:12 +0500, Andrey Borodin wrote:
>> 30 апр. 2018 г., в 0:39, Юрий Соколов  написал(а):
>>
>> (btw, why no one mention CLOCK+ in this thread).
>
> I do not know what CLOCK+ is. Can you, please, send me a reference.

Maybe he means CLOCK-Pro?

-- 
Thomas Munro
http://www.enterprisedb.com

Re: [RFC] Add an until-0 loop in psql

[Peter Eisentraut]

On 4/30/18 07:01, Daniel Verite wrote:
>> As of v11, DO blocks can do transactions. I think this will meet your needs.
> 
> They do support COMMIT and ROLLBACK in the current
> development tree, but not VACUUM as in Pierre's example.

Support for VACUUM can be added in the future.  There is no big problem
with it.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

nbtsort.c performs unneeded (though harmless) truncation

[Peter Geoghegan]

I noticed that we're calling _bt_nonkey_truncate() needlessly when a
minimum key is needed at the leftmost page on each level of the tree.
This was always a special case, and I think that it should remain as
one. Attached patch avoids unneeded truncations, while preserving the
generic BTreeTupleGetNAtts() assertions.

This isn't a correctness issue, and the extra overhead of unneeded
truncation should be negligible, but what we have now seems confusing
to me.

-- 
Peter Geoghegan


0001-Don-t-truncate-away-non-key-attributes-for-leftmost-.patch
Description: Binary data

Re: Support Python 3 tests under MSVC

[Peter Eisentraut]

On 4/30/18 15:52, Andrew Dunstan wrote:
> I'd like to apply these - they only affect the testing script
> vcregress.pl, so they should be very low risk.

In case there are concerns about maintaining a second copy of the
"mangle" script: The obvious answer is to rewrite that script in Python.
 I tried that once upon a time but it got too complicated.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: Patch missing from back branches

[Tom Lane]

Andrew Dunstan  writes:
> Well, the commit suggests that we should be guided by what the buildfarm
> did, and then we never followed that up. I'd say in the light of
> experience we should backpatch it to the remaining live branches.

There are other VS2015 patches that were not pushed back further than
9.5, e.g. 0fb54de9a.  So I don't think we should push this one further
back than that.  But it'd be consistent to put it in 9.5.

regards, tom lane

Re: Patch missing from back branches

[Andrew Dunstan]

On 04/26/2018 01:29 AM, Michael Paquier wrote:
> On Thu, Apr 26, 2018 at 09:06:09AM +0500, Haroon wrote:
>> Apparently the following patch[1]  got committed to head only (9.6 at the
>> time) and never made it into back branches i.e. 9.5, 9.4 and 9.3. Is it an
>> oversight ?
>>
>> [1]
>> http://git.postgresql.org/pg/commitdiff/868628e4fd44d75987d6c099ac63613cc5417629
> The support of VS 2015 being back-patched down to 9.5 is intentional,
> as the ideawhen integrating a new integration with visual studio is to
> patch HEAD plus the latest major version already released, at least
> that's the move respected for the last couple of years.
>
> Now I think that you are right: we should have this patch present as
> well in REL9_5_STABLE in order to be able to get the combination
> REL9_5_STABLE/VS2015 working correctly.  At least we have made efforts
> to claim support of VS2015 down to 9.5, so I would suggest to
> cherry-pick it and make the solution complete.
>
> The discussion about _timezone and _tzdata is here by the way:
> https://www.postgresql.org/message-id/CAB7nPqTEkqF7E7nWcdQSvyOUAV3OGjX%3DrKSxFTkk03TOJGF5ng%40mail.gmail.com
>
> There is dory in the buildfarm which compiles using VS 2015, but it runs
> only 9.6 and newer versions.
>
> Robert?  Or Magnus perhaps?


Well, the commit suggests that we should be guided by what the buildfarm
did, and then we never followed that up. I'd say in the light of
experience we should backpatch it to the remaining live branches.

cheers

andrew


-- 
Andrew Dunstanhttps://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Support Python 3 tests under MSVC

[Andrew Dunstan]

Here are a couple of patches, one for master and one for the back
branches from 9.5 to 10 to allow testing of plpython3 and associated
contrib modules when building with MSVC.

I've tested this out on a Windows 2016 machine with Visual Studio 2017.

I'd like to apply these - they only affect the testing script
vcregress.pl, so they should be very low risk.


cheers

andrew



-- 
Andrew Dunstanhttps://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index f24f975..5dc72ff 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -246,6 +246,53 @@ sub taptest
 	exit $status if $status;
 }
 
+sub mangle_plpython3
+{
+	my $tests = shift;
+	mkdir "results" unless -d "results";
+	mkdir "sql/python3";
+	mkdir "results/python3";
+	mkdir "expected/python3";
+
+	foreach my $test (@$tests)
+	{
+		local $/ = undef;
+		foreach my $dir ('sql','expected')
+		{
+			my $extension = ($dir eq 'sql' ? 'sql' : 'out');
+
+			my @files = glob("$dir/$test.$extension $dir/${test}_[0-9].$extension");
+			foreach my $file (@files)
+			{
+open(my $handle, "$file") || die "test file $file not found";
+my $contents = <$handle>;
+close($handle);
+map
+{
+	s/except ([[:alpha:]][[:alpha:].]*), *([[:alpha:]][[:alpha:]]*):/except $1 as $2:/g;
+	s///g;
+	s///g;
+	s/([0-9][0-9]*)L/$1/g;
+	s/([ [{])u"/$1"/g;
+	s/([ [{])u'/$1'/g;
+	s/def next/def __next__/g;
+	s/LANGUAGE plpython2?u/LANGUAGE plpython3u/g;
+	s/EXTENSION ([^ ]*_)*plpython2?u/EXTENSION $1plpython3u/g;
+	s/installing required extension "plpython2u"/installing required extension "plpython3u"/g;
+} $contents;
+my $base = basename $file;
+open($handle, ">$dir/python3/$base") || die "opening python 3 file for $file";
+print $handle $contents;
+close($handle);
+			}
+		}
+	}
+	map { $_ =~ s!^!python3/!; } @$tests;
+	return @$tests;
+}
+
+
+
 sub plcheck
 {
 	chdir "$topdir/src/pl";
@@ -268,7 +315,8 @@ sub plcheck
 		}
 		if ($lang eq 'plpython')
 		{
-			next unless -d "$topdir/$Config/plpython2";
+			next unless -d "$topdir/$Config/plpython2" ||
+-d "$topdir/$Config/plpython3";
 			$lang = 'plpythonu';
 		}
 		else
@@ -278,6 +326,8 @@ sub plcheck
 		my @lang_args = ("--load-extension=$lang");
 		chdir $dir;
 		my @tests = fetchTests();
+		@tests = mangle_plpython3(\@tests)
+			if $lang eq 'plpythonu' && -d "$topdir/$Config/plpython3";
 		if ($lang eq 'plperl')
 		{
 
@@ -293,6 +343,10 @@ sub plcheck
 push(@tests, 'plperl_plperlu');
 			}
 		}
+		elsif ($lang eq 'plpythonu' && -d "$topdir/$Config/plpython3")
+		{
+			@lang_args = ();
+		}
 		print
 		  "\n";
 		print "Checking $lang\n";
@@ -335,6 +389,8 @@ sub subdircheck
 		die "Python not enabled in configuration"
 		  if !defined($config->{python});
 
+		@opts = grep { $_ !~ /plpythonu/ } @opts;
+
 		# Attempt to get python version and location.
 		# Assume python.exe in specified dir.
 		my $pythonprog = "import sys;" . "print(str(sys.version_info[0]))";
@@ -349,10 +405,7 @@ sub subdircheck
 		}
 		else
 		{
-
-			# disable tests on python3 for now.
-			chdir "..";
-			return;
+			@tests = mangle_plpython3(\@tests);
 		}
 	}
 
@@ -363,6 +416,7 @@ sub subdircheck
 		"$topdir/$Config/pg_regress/pg_regress",
 		"--bindir=${topdir}/${Config}/psql",
 		"--dbname=contrib_regression", @opts, @tests);
+	print join(' ',@args),"\n";
 	system(@args);
 	chdir "..";
 }
@@ -378,11 +432,8 @@ sub contribcheck
 		next if ($module eq "uuid-ossp" && !defined($config->{uuid}));
 		next if ($module eq "sslinfo"   && !defined($config->{openssl}));
 		next if ($module eq "xml2"  && !defined($config->{xml}));
-		next if ($module eq "hstore_plperl" && !defined($config->{perl}));
-		next if ($module eq "jsonb_plperl"  && !defined($config->{perl}));
-		next if ($module eq "hstore_plpython" && !defined($config->{python}));
-		next if ($module eq "jsonb_plpython"  && !defined($config->{python}));
-		next if ($module eq "ltree_plpython"  && !defined($config->{python}));
+		next if ($module =~ /_plperl$/  && !defined($config->{perl}));
+		next if ($module =~ /_plpython$/&& !defined($config->{python}));
 		next if ($module eq "sepgsql");
 
 		subdircheck("$topdir/contrib", $module);
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index 2904679..43d5d76 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -244,6 +244,53 @@ sub taptest
 	exit $status if $status;
 }
 
+sub mangle_plpython3
+{
+	my $tests = shift;
+	mkdir "results" unless -d "results";
+	mkdir "sql/python3";
+	mkdir "results/python3";
+	mkdir "expected/python3";
+
+	foreach my $test (@$tests)
+	{
+		local $/ = undef;
+		foreach my $dir ('sql','expected')
+		{
+			my $extension = ($dir eq 'sql' ? 'sql' : 'out');
+
+			my @files = 

Re: "could not reattach to shared memory" on buildfarm member dory

[Tom Lane]

Heath Lord  writes:
>So what I noticed after adding the '--force' flag was that in the Event
> Viewer logs there was an Error in the System log stating that "The
> application-specific permission settings do not grant Local Activation
> permission for the COM Server application" for the Runtime Broker.  So
> around 2:00 pm today I went and changed the ownership of the registry
> values to Administrators so I could add the user we are running the builds
> under to the list of members that have access to it.  However right after I
> made that change the build was actually broken for me so I am just now
> turning the builds back on to run constantly to verify if this has
> any effect on the issue of not being able to reattach to the shared
> memory.  I am hoping that this makes things more stable, however I am not
> confident that these are related.

The build was broken on Windows between about 12:30 and 14:30 EDT,
thanks to an unrelated change.  Now that that's sorted, dory is
still failing :-(.

Moreover, even though I took out the module dump logic, the failure rate
is still a good bit higher than it was before, which seems to confirm my
fear that this is a Heisenbug: either VirtualQuery itself, or the act of
elog'ing a bunch of output, is causing memory allocations to take place
that otherwise would not have.

I'm hoping that the elog output is to blame for that, and am going to
go try to rejigger the code so that we capture the memory maps into space
that was allocated before VirtualFree.

>Any ideas or changes that we could do to help debug or verify would be
> helpful.  We have considered changing it to run everything as SYSTEM but if
> possible we would like to avoid this for security reasons.

Yeah, that's no solution.  Even if it were OK for dory, end users wouldn't
necessarily want to run PG that way.

regards, tom lane

Re: Issues while building PG in MS Windows, using MSYS2 and MinGW-w64

[Andrew Dunstan]

On 04/30/2018 02:59 PM, Tom Lane wrote:
> "insaf.k"  writes:
>> gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement 
>> -Wendif-labels -Wmissing-format-attribute -Wformat-security 
>> -fno-strict-aliasing -fwrapv -fexcess-precision=standard -g -std=c11 
>> -I../../src/port -DFRONTEND -I../../src/include  -I./src/include/port/win32 
>> -DEXEC_BACKEND  "-I../../src/include/port/win32" -DBUILDING_DLL  -c -o 
>> win32env.o win32env.c
>> win32env.c: In function 'pgwin32_putenv':
>> win32env.c:24:22: error: expected ')' before '*' token
>>   typedef int (_cdecl * PUTENVPROC) (const char *);
>>   ^
> Hmm.  This sort of looks like your compiler isn't recognizing _cdecl
> as a special token, but I have no idea why a Windows-ready compiler
> wouldn't handle that.
>
>   



I haven't looked deeply at this thread, but it's worth pointing out that
MSYS2 is not the same thing as MSYS, and I at least have no experience
of building with it.

Now Msys hasn't been updated for quite a while. so maybe we need to look
at these tools. Our documentation currently does not refer to MSYS2 at all.

cheers

andrew

-- 
Andrew Dunstanhttps://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: "could not reattach to shared memory" on buildfarm member dory

[Stephen Frost]

Greetings,

* Heath Lord (heath.l...@crunchydata.com) wrote:
>Any ideas or changes that we could do to help debug or verify would be
> helpful.  We have considered changing it to run everything as SYSTEM but if
> possible we would like to avoid this for security reasons.  Thank you in
> advance and I appreciate all the help.

Just to be clear- there is no longer anything showing up in the event
viewer associated with running the builds.  There may still be an issue
with the system setup or such, but it at least seems less likely for
that to be the issue, so I'm thinking that Tom is more likely correct
that PG is doing something not quite right here.

Thanks!

Stephen


signature.asc
Description: PGP signature

Re: "could not reattach to shared memory" on buildfarm member dory

[Heath Lord]

On Mon, Apr 30, 2018 at 2:34 PM, Stephen Frost  wrote:

> Greetings,
>
> * Tom Lane (t...@sss.pgh.pa.us) wrote:
> > [ Thanks to Stephen for cranking up a continuous build loop on dory ]
>
> That was actually Heath, who is also trying to work this issue and has
> an idea about something else which might help (and some more information
> about what's happening in the event log).  Adding him to the thread so
> he can (easily) reply with what he's found.
>
> Heath..?
>
> Thanks!
>
> Stephen
>

   So what I noticed after adding the '--force' flag was that in the Event
Viewer logs there was an Error in the System log stating that "The
application-specific permission settings do not grant Local Activation
permission for the COM Server application" for the Runtime Broker.  So
around 2:00 pm today I went and changed the ownership of the registry
values to Administrators so I could add the user we are running the builds
under to the list of members that have access to it.  However right after I
made that change the build was actually broken for me so I am just now
turning the builds back on to run constantly to verify if this has
any effect on the issue of not being able to reattach to the shared
memory.  I am hoping that this makes things more stable, however I am not
confident that these are related.
  The change that I attempted prior to this, which was done last Wednesday,
was to remove the vctip.exe program from being used as this was causing
issues for us as well.  This was causing an Event ID 1530 error that stated
that "The application this is listed in the event details is leaving the
registry handle open" and Windows was helpfully closing any registry values
for that user profile, and I thought that possibly when doing so it was
cleaning up the memory that was allocated prior.  However this did not
change anything for us after making that change.
   Any ideas or changes that we could do to help debug or verify would be
helpful.  We have considered changing it to run everything as SYSTEM but if
possible we would like to avoid this for security reasons.  Thank you in
advance and I appreciate all the help.

-Heath

Re: mogrify and indent features for jsonb

[Andrew Dunstan]

On 04/30/2018 05:33 AM, jamesmalvi wrote:
> I would like to suggest https://jsonformatter.org/json-pretty-print for
> formatting and beautifying JSON data. 
>
> Also for use this validationg JSON data, https://jsonformatter.org 
>
>


We already have a pretty-print function built in, and we already use a
validating parser. So it's not clear to me what you want that we don't have.


cheers

andrew

-- 
Andrew Dunstanhttps://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: documentation is now XML

[Peter Eisentraut]

On 4/27/18 11:03, Bruce Momjian wrote:
> On Fri, Apr 27, 2018 at 11:00:36AM -0400, Peter Eisentraut wrote:
>> On 4/23/18 05:54, Liudmila Mantrova wrote:
>>> Reading this thread, I got an impression that everyone would benefit
>>> from converting back branches to XML, but the main concern is lack of
>>> resources to complete this task. Are there any other issues that affect
>>> this decision? Looks like Aleksander Lakhin's offer to prepare patches
>>> was missed somehow as the discussion sidetracked to other issues
>>
>> That proposal seemed to indicate not only converting the source code to
>> XML but also the build system to XSL.  The latter is out of the
>> question, I think.
> 
> Why is that?
Because there would be a thousand lines of tooling changes to be
backported and thousands of pages of documentation to be checked
manually that it doesn't create a mess (times branches times platforms).

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: obsoleting plpython2u and defaulting plpythonu to plpython3u

[Peter Eisentraut]

On 4/27/18 12:38, Pavel Raiskup wrote:
> Well, also it depends what's meant by "get PEP 394 changed".  My guess is
> that distros might well avoid providing /bin/python if that PEP is
> a concern..  that's actually typical situation in Fedora even now; python2
> package isn't installed by default.  And it can disappear entirely if
> that's too expensive for maintenance.

I'm wondering who this situation is going to be resolved.  Is anyone
going to switch /usr/bin/python to Python 3 in the foreseeable future?
A few have tried and regretted it.

Or is /usr/bin/python just going to go away when Python 2 goes away?

What we do in PostgreSQL would IMO depend on which of these choices is
going to be the dominant one.

If the former, then we can just switch plpythonu around to be Python 3
(perhaps a configure option).

If the latter, then we don't need to do anything, except perhaps warn
people to upgrade.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: [HACKERS] Add hint for function named "is"

[Robert Haas]

On Mon, Apr 30, 2018 at 2:14 PM, Tom Lane  wrote:
> Not *nearly* as much as I'd object to mostly-breaking postfix operators.
> Now admittedly, if the Berkeley guys had never put those in, nobody
> would miss them.  But they're there, and I'm afraid that users are
> likely depending on them.  Neither of your suggestions above would be
> any less damaging to such users than removing the feature altogether.

Well, that sounds different than what you said upthread in August of 2016:

tgl> Agreed, if postfix operators were the only thing standing between us and
tgl> fixing that, it would be a pretty strong argument for removing them.

I agree that there could be some people using numeric_fac, the only
postfix operator we ship.  Probably not many, since it's an
inefficient implementation of an operation that overflows for all but
a few tens of thousands of possible input values, but some.  But we
could keep that operator working without supporting the facility in
general.  Do we have any evidence at all that anybody has got a
user-defined postfix operator out there?  I'm reluctant to keep a
feature that's blocking such an important improvement unless we can
come up with some real examples of it being used.  I've never run
across it, and a Google search didn't turn up any examples of anybody
else using it, either.

The basic problem here, for those following along from home, is that
if we allow "SELECT h hour FROM tab" to mean select column h with
column alias hour, then "SELECT hour + 24 * day" becomes ambiguous in
the face of postfix operators: it could either mean what it obviously
is intended to mean, or it could mean that you should look for a
postfix * operator, apply that to 24, add hour to the result, and then
call the resulting column "day".  This is less obviously insane when
you use a differently-named operator and columns -- who knows which
way "SELECT thunk + squidge!!! snarkke FROM frobnitz" is intended to
be interpreted.  But I'm just a bit doubtful that anyone is going to
stop using PostgreSQL because we take away their ability to make !!! a
postfix operator, whereas I think the inability to alias columns
without using AS when the column label happens to be a keyword IS a
problem for adoption.

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

Re: BufFileSize() doesn't work on a "shared" BufFiles

[Peter Geoghegan]

On Mon, Apr 30, 2018 at 10:18 AM, Heikki Linnakangas  wrote:
> Perhaps that would be OK, if it was properly commented. But it's not
> actually hard to make BufFileSize() work on shared files, too, so I think we
> should do that.

I agree that this could use a comment, but I don't see much point in
adding the extra FileSeek(). The fact that fd.c is unwilling to track
filesize for non-temp files (where "non-temp" means a
PathNameOpenTemporaryFile()-returned/exported file) is due to
vfdP->fileSize being involved in temp_file_limit enforcement. Maybe a
FD_TEMP_FILE_LIMIT assertion within FileGetSize() would help?

> Another little bug I noticed is that BufFileAppend() incorrectly resets the
> 'offsets' of the source files. You won't notice if you call BufFileAppend()
> immediately after BufFileOpenShared(), but if you had called BufFileSeek()
> or BufFileRead() on the shared BufFile handle before calling
> BufFileAppend(), it would get confused.

Seems worth fixing.

-- 
Peter Geoghegan

Re: Issues while building PG in MS Windows, using MSYS2 and MinGW-w64

[Tom Lane]

"insaf.k"  writes:
> gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement 
> -Wendif-labels -Wmissing-format-attribute -Wformat-security 
> -fno-strict-aliasing -fwrapv -fexcess-precision=standard -g -std=c11 
> -I../../src/port -DFRONTEND -I../../src/include  -I./src/include/port/win32 
> -DEXEC_BACKEND  "-I../../src/include/port/win32" -DBUILDING_DLL  -c -o 
> win32env.o win32env.c

> win32env.c: In function 'pgwin32_putenv':
> win32env.c:24:22: error: expected ')' before '*' token
>   typedef int (_cdecl * PUTENVPROC) (const char *);
>   ^

Hmm.  This sort of looks like your compiler isn't recognizing _cdecl
as a special token, but I have no idea why a Windows-ready compiler
wouldn't handle that.

regards, tom lane

Re: [RFC] Add an until-0 loop in psql

[Corey Huinker]

On Mon, Apr 30, 2018 at 7:05 AM Pierre Ducroquet <
pierre.ducroq...@people-doc.com> wrote:

> On Monday, April 30, 2018 1:01:25 PM CEST Daniel Verite wrote:
> >   Corey Huinker wrote:
> > > As of v11, DO blocks can do transactions. I think this will meet your
> > > needs.
> > They do support COMMIT and ROLLBACK in the current
> > development tree, but not VACUUM as in Pierre's example.
> >
> > postgres=# \echo :SERVER_VERSION_NAME
> > 11devel
> >
> > postgres=# do ' begin vacuum; end ';
> > ERROR:VACUUM cannot be executed from a function
> > CONTEXT:  SQL statement "vacuum"
> > PL/pgSQL function inline_code_block line 1 at SQL statement
> >
> >
> > Best regards,
>
> Indeed, vacuum is going to be the biggest offender here, sadly.
> One could work around this of course (on top of my head, using notify to
> wake-
> up another client that would launch the required vacuums…)
> Being able to do transactions in DO blocks is a great new feature of v11 I
> was
> not aware of. But psql saw the addition of \if recently, so why not having
> loops in there too ? (Something better than this hack of course, it was
> just a
> 10 minutes hack-sprint for a demo)
>
> Regards
>
>  Pierre
>

Bummer about vacuum.

If you dig into the very long discussion about \if (which, incidentally,
started off as a 20-line command patch called \quit-if, so don't discount
that your idea could take off), you'll see some of the problems with
looping discussed, mostly about the issues I already alluded to (no concept
of reading backwards on STDIN, scoping outside the current "file", ability
of psql vars to contain executable \commands), you'll have a pretty good
grasp of the places where psql would need changes.

In the mean time, if you believe the table won't get much larger during the
operation, you could use \gexec as a finite loop iterator

SELECT count(*)::bigint / 1000 FROM big_table as num_iters
\gset
SELECT
'BEGIN',
'DELETE FROM big_table WHERE id IN (SELECT id FROM big_table WHERE bad
= true LIMIT 1000)',
'VACUUM big_table',
'COMMIT'
from generate_series(1,:num_iters) g
\gexec


If the number of rows increases, then your finite loop will fall short, and
if something else deletes a bunch of rows, your loop will spin it's wheels
a few times at the end, but it would do most of what you want.

Re: "could not reattach to shared memory" on buildfarm member dory

[Stephen Frost]

Greetings,

* Tom Lane (t...@sss.pgh.pa.us) wrote:
> [ Thanks to Stephen for cranking up a continuous build loop on dory ]

That was actually Heath, who is also trying to work this issue and has
an idea about something else which might help (and some more information
about what's happening in the event log).  Adding him to the thread so
he can (easily) reply with what he's found.

Heath..?

Thanks!

Stephen


signature.asc
Description: PGP signature

Re: Verbosity of genbki.pl

[Peter Eisentraut]

On 4/29/18 06:50, Michael Paquier wrote:
> On Sat, Apr 28, 2018 at 01:02:10PM -0400, Stephen Frost wrote:
>> +1 for making them not output anything if all is well.
> 
> +1.

done

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: [HACKERS] Clock with Adaptive Replacement

[Andres Freund]

Hi,

On 2018-04-30 15:39:08 +0500, Andrey Borodin wrote:
> I suspect that performance benefits can be not that big or even
> marginal, if we do not extend comprehensive eviction strategy with
> bgwriter fixes and O_DIRECT.

If so, then the improvements aren't real. Bgwriter doesn't matter for
read-only workloads. O_DIRECT doesn't matter much if shared_buffers are
60+% of OS memory.  And even disregarding that, you can just compute
cache hit ratios to see whether things are improving.

Greetings,

Andres Freund

Re: [HACKERS] Add hint for function named "is"

[Tom Lane]

Robert Haas  writes:
> I think there would be a lot of value in coming up with some kind of
> incremental improvement here; this is a common annoyance for users
> migrating from other database systems (and one in particular).

Agreed, but ...

> Technically, that doesn't look hard to do: (1) remove the rule that
> allows postfix ops, or restrict it to operators beginning with ! or
> where OPERATOR() notation is used, or whatever; (2) add a new
> production target_el_keyword that includes some or all of the keywords
> that don't cause grammar conflicts, (3) add a rule that target_el can
> be "a expr target_el_keyword", (4) profit.  Or, since that would make
> maintaining target_el_keyword a nuisance, split unreserved_keyword
> into two new categories, unreserved_keyword and
> very_slightly_reserved_keyword, and update elsewhere accordingly.
> However, I foresee that Tom will object to the idea of creating a new
> category of keywords, and I'm happy to do something else if we can
> figure out what that other thing is.

Not *nearly* as much as I'd object to mostly-breaking postfix operators.
Now admittedly, if the Berkeley guys had never put those in, nobody
would miss them.  But they're there, and I'm afraid that users are
likely depending on them.  Neither of your suggestions above would be
any less damaging to such users than removing the feature altogether.

> I'm not immediately sure how to
> use operator precedence to resolve these ambiguities;

Unfortunately, this thread got swapped out of my brain long ago,
so I'm not sure what I was talking about either.  I could take
another look sometime.

regards, tom lane

Re: [HACKERS] Add hint for function named "is"

[Robert Haas]

On Fri, Aug 12, 2016 at 2:40 PM, Tom Lane  wrote:
> It's likely that by rejiggering the precedence of productions, we could
> resolve these ambiguities in favor of "it's not a ColLabel if it appears
> in a context like this".  And probably that wouldn't be too surprising
> most of the time.  But depending on precedence to resolve fundamentally
> ambiguous grammar is always gonna bite you sometimes.  People understand
> it (... usually ...) in the context of parsing multi-operator expressions,
> but for other things it's not a great tool.

I poked at this a little more today.  It might be a little easier to
shoot for changing "a_expr IDENT" to "a_expr ColId" than to shoot for
allowing "a_expr ColLabel", or even just allowing both "a_expr IDENT"
and "a_expr unreserved_keyword".  With the rule for postfix operators
is removed, following unreserved keywords are problematic: DAY FILTER
HOUR MINUTE MONTH OVER SECOND VARYING WITHIN WITHOUT YEAR.  I think
allowing ColId would create further problems with PRECISION, CHAR, and
CHARACTER as well.  But they are all shift/reduce conflicts, which
somehow scare me quite a bit less than the reduce/reduce conflicts one
gets when trying to allow ColLabel.

I think there would be a lot of value in coming up with some kind of
incremental improvement here; this is a common annoyance for users
migrating from other database systems (and one in particular).  We
currently have 440 keywords of which 290 are unreserved, 50 are
column-name keywords, 23 are type-func-name keywords, and 77 are fully
reserved.  For more than 300 of those key words, the postfix operator
rule is the only thing preventing us from allowing it as a column
label without "AS".  Eliminating just those -- or even a large subset
of those -- would make things noticeably better, IMHO.

Technically, that doesn't look hard to do: (1) remove the rule that
allows postfix ops, or restrict it to operators beginning with ! or
where OPERATOR() notation is used, or whatever; (2) add a new
production target_el_keyword that includes some or all of the keywords
that don't cause grammar conflicts, (3) add a rule that target_el can
be "a expr target_el_keyword", (4) profit.  Or, since that would make
maintaining target_el_keyword a nuisance, split unreserved_keyword
into two new categories, unreserved_keyword and
very_slightly_reserved_keyword, and update elsewhere accordingly.
However, I foresee that Tom will object to the idea of creating a new
category of keywords, and I'm happy to do something else if we can
figure out what that other thing is.  I'm not immediately sure how to
use operator precedence to resolve these ambiguities; I think that for
that to work we'd have assign a precedence to every keyword that we
want to allow here, just as we currently do for IDENT.  And that seems
like it could have unforeseen side effects.

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

Re: mogrify and indent features for jsonb

[jamesmalvi]

I would like to suggest https://jsonformatter.org/json-pretty-print for
formatting and beautifying JSON data. 

Also for use this validationg JSON data, https://jsonformatter.org 



--
Sent from: http://www.postgresql-archive.org/PostgreSQL-hackers-f1928748.html

Re: Issues while building PG in MS Windows, using MSYS2 and MinGW-w64

[insaf.k]

Hi Tom Lane, 





Thanks for the help.



I had extracted PG using WinZip outside of MSYS2, and it was causing many of 
the errors, may be due to some errors in file attributes. So I unpacked the 
source tarball within the shell itself(using gunzip and tar), and I could fix 
many errors.



Now new errors are popping up. I've compared the config file with the file 
given in the URL, but I couldn't fix this yet.



The current error is basically due to the fact that, PUTENVPROC macro is 
defined to empty.



Config output and make error output are uploaded here 
https://gist.github.com/codercet/760ae3b0337144059570ccfc06e2a847



config.log is available here 
https://gist.github.com/codercet/725cc5fdd0acfbf5193d891bffb4dfda



The error output while doing make is



gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement 
-Wendif-labels -Wmissing-format-attribute -Wformat-security 
-fno-strict-aliasing -fwrapv -fexcess-precision=standard -g -std=c11 
-I../../src/port -DFRONTEND -I../../src/include  -I./src/include/port/win32 
-DEXEC_BACKEND  "-I../../src/include/port/win32" -DBUILDING_DLL  -c -o 
win32env.o win32env.c

win32env.c: In function 'pgwin32_putenv':

win32env.c:24:22: error: expected ')' before '*' token

  typedef int (_cdecl * PUTENVPROC) (const char *);

  ^

win32env.c:96:4: error: unknown type name 'PUTENVPROC'; did you mean 
'IMCENUMPROC'?

PUTENVPROC putenvFunc;

^~

IMCENUMPROC

win32env.c:98:18: error: 'PUTENVPROC' undeclared (first use in this function); 
did you mean 'IMCENUMPROC'?

putenvFunc = (PUTENVPROC) GetProcAddress(hmodule, "_putenv");

  ^~

  IMCENUMPROC

win32env.c:98:18: note: each undeclared identifier is reported only once for 
each function it appears in

win32env.c:98:30: error: expected ';' before 'GetProcAddress'

putenvFunc = (PUTENVPROC) GetProcAddress(hmodule, "_putenv");

  ^~

win32env.c:100:5: error: called object 'putenvFunc' is not a function or 
function pointer

 putenvFunc(envval);

 ^~

win32env.c:96:15: note: declared here

PUTENVPROC putenvFunc;

   ^~

make[2]: *** [builtin: win32env.o] Error 1

make[2]: Leaving directory '/home/admin/postgresql-10.0/src/port'

make[1]: *** [Makefile:37: all-port-recurse] Error 2

make[1]: Leaving directory '/home/admin/postgresql-10.0/src'

make: *** [GNUmakefile:11: all-src-recurse] Error 2





Could you please help?



Best Regards,

Mohamed Insaf K

 On Fri, 27 Apr 2018 21:01:56 +0530 Tom Lane t...@sss.pgh.pa.us 
wrote 




"insaf.k" insa...@zohocorp.com writes: 

 I am trying to build PG from source, in MS Windows using MSYS2 and 
MinGW-w64. I've tried to build PG 10.0 as wells as 10.3. 

 I've done configuring like this 

 ./configure --prefix="/d/pg10/" 

 And when I do "make" or "make world", I'm getting compilation error. I've 
attached complete error report at the end of the mail. 

 

I don't know anything about mingw, but from the first error message you 

showed, I'd venture that configure failed to fill in pg_config_ext.h 

correctly. It should have put in a line like 

 

#define PG_INT64_TYPE long long int 

 

and evidently it hasn't. This suggests that there's something wrong 

with one of the text-processing tools it uses, such as "sed". You 

might look into config.log and see if there are any relevant-looking 

error messages (probably down near the end). Also, try comparing 

your configure text output and config.log to those of one of our 

buildfarm machines that's using mingw successfully, such as 

 

https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=jacanadt=2018-04-27%2012%3A19%3A49
 

 

(the "config" and "configure" links on that page are what to compare to). 

 

Hm ... I notice that jacana's been set up so that it explicitly 

gives configure a --host setting instead of letting configure work 

that out. No idea if that was really necessary or not. 

 

regards, tom lane 

Re: proposal: schema variables

[Fabrízio Mello]

The following review has been posted through the commitfest application:
make installcheck-world:  tested, passed
Implements feature:   not tested
Spec compliant:   not tested
Documentation:not tested

1) There are some errors applying the patch against the current master:

fabrizio@macanudo:/d/postgresql (master) 
$ git apply /tmp/schema-variables-poc-180429-01-diff
/tmp/schema-variables-poc-180429-01-diff:2305: trailing whitespace.
 
/tmp/schema-variables-poc-180429-01-diff:2317: indent with spaces.
We can support UPDATE and SELECT commands on variables.
/tmp/schema-variables-poc-180429-01-diff:2319: indent with spaces.
possible syntaxes:
/tmp/schema-variables-poc-180429-01-diff:2321: indent with spaces.
-- there can be a analogy with record functions
/tmp/schema-variables-poc-180429-01-diff:2322: indent with spaces.
SELECT varname;
warning: squelched 14 whitespace errors
warning: 19 lines add whitespace errors.


2) There are some warnings when during build process

schemavar.c:383:18: warning: expression which evaluates to zero treated as a 
null pointer constant of type 'HeapTuple' (aka 'struct HeapTupleData *')
  [-Wnon-literal-null-conversion]
HeapTuple   tp = InvalidOid;
 ^~
../../../src/include/postgres_ext.h:36:21: note: expanded from macro 
'InvalidOid'
#define InvalidOid  ((Oid) 0)
^
1 warning generated.
tab-complete.c:1268:21: warning: initialization from incompatible pointer type 
[-Wincompatible-pointer-types]
  {"VARIABLE", NULL, _for_list_of_variables},
 ^
tab-complete.c:1268:21: note: (near initialization for 
‘words_after_create[48].vquery’)
llvmjit_expr.c: In function ‘llvm_compile_expr’:
llvmjit_expr.c:253:3: warning: enumeration value ‘EEOP_PARAM_SCHEMA_VARIABLE’ 
not handled in switch [-Wswitch]
   switch (opcode)
   ^

Re: Transform for pl/perl

[Peter Eisentraut]

These two items are now outstanding:

On 4/10/18 07:33, Dagfinn Ilmari Mannsåker wrote:
> 2) jsonb scalar values are passed to the plperl function wrapped in not
>one, but _two_ layers of references

I don't understand this one, or why it's a problem, or what to do about it.

> 3) jsonb numeric values are passed as perl's NV (floating point) type,
>losing precision if they're integers that would fit in an IV or UV.

This seems fixable, but perhaps we need to think through whether this
will result in other strange behaviors.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: Transform for pl/perl

[Peter Eisentraut]

On 4/29/18 14:28, Tom Lane wrote:
> Peter Eisentraut  writes:
>> On 4/24/18 14:31, Andrew Dunstan wrote:
>>> There is the routine IsValidJsonNumber that helps - see among others
>>> hstore_io.c for an example use.
> 
>> I would need something like that taking a double/float8 input.  But I
>> think there is no such shortcut available, so I just wrote out the tests
>> for isinf and isnan explicitly.  Attached patch should fix it.
>> jsonb_plpython will need a similar fix.
> 
> I looked this over, it looks fine to me.  I first questioned the use
> of ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE for rejecting NaN, but I see
> that we are doing that in lots of comparable places (e.g., dtoi4())
> so I'm on board with it.

Yeah, that was the idea.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

BufFileSize() doesn't work on a "shared" BufFiles

[Heikki Linnakangas]

Hi,

I've been noodling around the tuplesort/logtape code, reviewing the 
changes that were made in v11 to support parallel sorting, and also 
rebasing my older patch to replace the polyphase merge with a simple 
balanced merge.


Looking at TapeShare:


/*
 * The approach tuplesort.c takes to parallel external sorts is that workers,
 * whose state is almost the same as independent serial sorts, are made to
 * produce a final materialized tape of sorted output in all cases.  This is
 * frozen, just like any case requiring a final materialized tape.  However,
 * there is one difference, which is that freezing will also export an
 * underlying shared fileset BufFile for sharing.  Freezing produces TapeShare
 * metadata for the worker when this happens, which is passed along through
 * shared memory to leader.
 *
 * The leader process can then pass an array of TapeShare metadata (one per
 * worker participant) to LogicalTapeSetCreate(), alongside a handle to a
 * shared fileset, which is sufficient to construct a new logical tapeset that
 * consists of each of the tapes materialized by workers.
 *
 * Note that while logtape.c does create an empty leader tape at the end of the
 * tapeset in the leader case, it can never be written to due to a restriction
 * in the shared buffile infrastructure.
 */
typedef struct TapeShare
{
/*
 * firstblocknumber is first block that should be read from materialized
 * tape.
 *
 * buffilesize is the size of associated BufFile following freezing.
 */
longfirstblocknumber;
off_t   buffilesize;
} TapeShare;


Why is 'buffilesize' part of the exported state? The leader process can 
easily call BufFileSize() itself, instead of having the worker process 
pass it through shared memory, right? Wrong. I tried to change it that 
way, and after some debugging, I realized that BufFileSize() doesn't 
work if it's called on a "shared" BufFile. It always returns 0. That's 
because it uses FileGetSize(), which in turn only works on a temporary 
file. None of this is mentioned in the comments :-(.


Perhaps that would be OK, if it was properly commented. But it's not 
actually hard to make BufFileSize() work on shared files, too, so I 
think we should do that.


Another little bug I noticed is that BufFileAppend() incorrectly resets 
the 'offsets' of the source files. You won't notice if you call 
BufFileAppend() immediately after BufFileOpenShared(), but if you had 
called BufFileSeek() or BufFileRead() on the shared BufFile handle 
before calling BufFileAppend(), it would get confused.


I propose the attached patch.

- Heikki
>From 6d8c1619c448bc66351eb51393950f898b5c3c5f Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas 
Date: Mon, 30 Apr 2018 20:05:59 +0300
Subject: [PATCH 1/1] Fix some sloppiness in the new BufFileSize() and
 BufFileAppend() funtions.

There were three related issues:

* BufFileAppend() incorrectly reset the seek position on the 'source'
  file. As a result, if you had called BufFileRead() on the file before
  calling BufFileAppend(), it got confused, and subsequent calls would
  read/write at wrong position.

* BufFileSize() did not work with files opened with BufFileOpenShared().

* FileGetSize() only worked on temporary files.

To fix, change the way BufFileSize() works so that it works on shared
files. Remove FileGetSize() altogetherm, as it's no longer needed. Remove
buffilesize from TapeShare struct, as the leader process can simply call
BufFileSize() to get the tape's size, there's no need to pass it through
shared memory.
---
 src/backend/storage/file/buffile.c | 18 ++
 src/backend/storage/file/fd.c  | 10 --
 src/backend/utils/sort/logtape.c   | 11 ---
 src/backend/utils/sort/tuplesort.c |  1 -
 src/include/storage/fd.h   |  1 -
 src/include/utils/logtape.h|  7 ++-
 6 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/src/backend/storage/file/buffile.c b/src/backend/storage/file/buffile.c
index 9cdddba510..d8a18dd3dc 100644
--- a/src/backend/storage/file/buffile.c
+++ b/src/backend/storage/file/buffile.c
@@ -802,14 +802,24 @@ BufFileTellBlock(BufFile *file)
 #endif
 
 /*
- * Return the current file size.  Counts any holes left behind by
- * BufFileViewAppend as part of the size.
+ * Return the current file size.
+ *
+ * Counts any holes left behind by BufFileAppend as part of the size.
+ * Returns -1 on error.
  */
 off_t
 BufFileSize(BufFile *file)
 {
+	off_t		lastFileSize;
+
+	/* Get the size of the last physical file by seeking to end. */
+	lastFileSize = FileSeek(file->files[file->numFiles - 1], 0, SEEK_END);
+	if (lastFileSize < 0)
+		return -1;
+	file->offsets[file->numFiles - 1] = lastFileSize;
+
 	return ((file->numFiles - 1) * (off_t) MAX_PHYSICAL_FILESIZE) +
-		FileGetSize(file->files[file->numFiles - 1]);
+		lastFileSize;
 }
 
 /*
@@ -853,7 +863,7 @@ BufFileAppend(BufFile 

Re: "could not reattach to shared memory" on buildfarm member dory

[Tom Lane]

[ Thanks to Stephen for cranking up a continuous build loop on dory ]

Noah Misch  writes:
> On Tue, Apr 24, 2018 at 11:37:33AM +1200, Thomas Munro wrote:
>> Maybe try asking what's mapped there with VirtualQueryEx() on failure?

> +1.  An implementation of that:
> https://www.postgresql.org/message-id/20170403065106.GA2624300%40tornado.leadboat.com

So I tried putting in that code, and it turns the problem from something
that maybe happens in every third buildfarm run or so, to something that
happens at least a dozen times in a single "make check" step.  This seems
to mean that either EnumProcessModules or GetModuleFileNameEx is itself
allocating memory, and sometimes that allocation comes out of the space
VirtualFree just freed :-(.

So we can't use those functions.  We have however proven that no new
module gets loaded during VirtualFree or MapViewOfFileEx, so there
doesn't seem to be anything more to be learned from them anyway.

What it looks like to me is that MapViewOfFileEx allocates some memory and
sometimes that comes out of the wrong place.  This is, um, unfortunate.
It also appears that VirtualFree might sometimes allocate some memory,
and that'd be even more unfortunate, but it's hard to be certain; the
blame might well fail on VirtualQuery instead.  (Ain't Heisenbugs fun?)

The solution I was thinking about last night was to have
PGSharedMemoryReAttach call MapViewOfFileEx to map the shared memory
segment at an unspecified address, then unmap it, then call VirtualFree,
and finally call MapViewOfFileEx with the real target address.  The idea
here is to get these various DLLs to set up any memory allocation pools
they're going to set up before we risk doing VirtualFree.  I am not,
at this point, convinced this will fix it :-( ... but I'm not sure what
else to try.

In any case, it's still pretty unclear why dory is showing this problem
and other buildfarm members are not.  whelk for instance seems to be
loading all the same DLLs and more besides.

regards, tom lane

Re: Postgres, fsync, and OSs (specifically linux)

[Andres Freund]

On 2018-04-30 13:03:24 +0800, Craig Ringer wrote:
> Hrm, something else that just came up. On 9.6+ we use sync_file_range.
> It's surely going to eat errors:
> 
> rc = sync_file_range(fd, offset, nbytes,
>  SYNC_FILE_RANGE_WRITE);
> 
> /* don't error out, this is just a performance optimization */
> if (rc != 0)
> {
> ereport(WARNING,
> (errcode_for_file_access(),
>  errmsg("could not flush dirty data: %m")));
> }

It's not. Only SYNC_FILE_RANGE_WAIT_{BEFORE,AFTER} eat errors. Which
seems sensible, because they could be considered data integrity
operations.

fs/sync.c:
SYSCALL_DEFINE4(sync_file_range, int, fd, loff_t, offset, loff_t, nbytes,
unsigned int, flags)
{
...

if (flags & SYNC_FILE_RANGE_WAIT_BEFORE) {
ret = file_fdatawait_range(f.file, offset, endbyte);
if (ret < 0)
goto out_put;
}

if (flags & SYNC_FILE_RANGE_WRITE) {
ret = __filemap_fdatawrite_range(mapping, offset, endbyte,
 WB_SYNC_NONE);
if (ret < 0)
goto out_put;
}

if (flags & SYNC_FILE_RANGE_WAIT_AFTER)
ret = file_fdatawait_range(f.file, offset, endbyte);


where
int file_fdatawait_range(struct file *file, loff_t start_byte, loff_t end_byte)
{
struct address_space *mapping = file->f_mapping;

__filemap_fdatawait_range(mapping, start_byte, end_byte);
return file_check_and_advance_wb_err(file);
}
EXPORT_SYMBOL(file_fdatawait_range);

the critical call is file_check_and_advance_wb_err(). That's the call
that checks and clears errors.

Would be good to add a kernel xfstest (gets used on all relevant FS
these days), to make sure that doesn't change.


> I'm very suspicious about the safety of the msync() path too.

That seems justified however:

SYSCALL_DEFINE3(msync, unsigned long, start, size_t, len, int, flags)
{
...
error = vfs_fsync_range(file, fstart, fend, 1);
..


 */
int vfs_fsync_range(struct file *file, loff_t start, loff_t end, int datasync)
{
...
return file->f_op->fsync(file, start, end, datasync);
}
EXPORT_SYMBOL(vfs_fsync_range);



int ext4_sync_file(struct file *file, loff_t start, loff_t end, int datasync)
{
...
ret = file_write_and_wait_range(file, start, end);
if (ret)
return ret;
...


STATIC int
xfs_file_fsync(
struct file *file,
loff_t  start,
loff_t  end,
int datasync)
{
...
error = file_write_and_wait_range(file, start, end);
if (error)
return error;


int file_write_and_wait_range(struct file *file, loff_t lstart, loff_t lend)
{
...
err2 = file_check_and_advance_wb_err(file);
if (!err)
err = err2;
return err;
}


Greetings,

Andres Freund

Re: Intermittent ECPG test failures on Windows buildfarm machines

[Andrew Dunstan]

On 04/30/2018 12:22 AM, Tom Lane wrote:
> Observe the following buildfarm failures:
>
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=whelk=2018-03-29%2013%3A41%3A13
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=woodlouse=2018-04-18%2016%3A42%3A03
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=dory=2018-04-27%2016%3A15%3A25
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=dory=2018-04-30%2000%3A45%3A25
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=dory=2018-04-30%2002%3A00%3A25
>
> The common feature is that a single ECPG test case emitted an empty stdout
> file.  There is no other indication of a problem: the corresponding stderr
> output files are correct (and no, the "correct" contents of those aren't
> empty), the test process exited with status zero, and there's no sign of
> an issue in the postmaster log.  And it's a different test case each time.
>
> I trawled the buildfarm logs back to the beginning of 2017 and can't find
> any similar failures before these.  So it seems like we broke something
> fairly recently, probably during March; but what?  The woodlouse case is
> on 9.6 not HEAD, suggesting that whatever we did wrong got back-patched,
> and for sure I see no recent commits in relevant-seeming code in 9.6.
>
> Baffled ... any ideas?
>
>   



I have long had problems along these lines on Cygwin, and occasionally
worse, where the ecpg tests would hang on Cygwin. I have never been able
to discover any rhyme or reason for it, so on lorikeet and the retiring
brolga the ecpg tests are just disabled. If Cygwin were a more important
platform I would have put more effort into diagnosing the problem.

But I don't recall having similar issues on other Windows configurations.

cheers

andrew

-- 
Andrew Dunstanhttps://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: Intermittent ECPG test failures on Windows buildfarm machines

[Michael Meskes]

Am Montag, den 30.04.2018, 00:22 -0400 schrieb Tom Lane:
> Observe the following buildfarm failures:
> ...
> The common feature is that a single ECPG test case emitted an empty
> stdout
> file.  There is no other indication of a problem: the corresponding
> stderr
> output files are correct (and no, the "correct" contents of those
> aren't
> empty), the test process exited with status zero, and there's no sign
> of
> an issue in the postmaster log.  And it's a different test case each
> time.
> 
> Baffled ... any ideas?

AFAICT there were like 4 commits to ecpg in March that were also
backported to 9.6. And while some included changes to the test suite I
have no idea which, if any, might result in this kind of problem. Also
there was at least one change to the Windows build system that impacted
ecpg.

Is there anyone out there with a Windows system who could bisect the
tree and find which commit is the culprit? Or did we have any changes
to the buildfarm scripts that may be causing this?

Michael
-- 
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org
Jabber: michael at xmpp dot meskes dot org
VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL

Re: Explain buffers wrong counter with parallel plans

[Adrien Nayrat]

On 04/29/2018 05:10 PM, Amit Kapila wrote:
> Yeah, it would have been convenient, if Gather and Finalize Aggregate
> displays that way as it would have been easier for users to
> understand.  However, as of now, the aggregated stats for parallel
> workers and leader are displayed at parallel/partial nodes as is
> displayed in the plan.  So according to me, what you are seeing is
> okay, it is only slightly tricky to understand it.
> 
[...]
> 
>> I played with git bisect and I found this commit :
>>
>> commit 01edb5c7fc3bcf6aea15f2b3be36189b52ad9d1a
>> Author: Tom Lane 
>> Date:   Fri Sep 1 17:38:54 2017 -0400
>>
> I think you were seeing different results before this commit because
> before that we were shutting down workers as soon as parallel workers
> are done and the buffer_usage stats were accumulated and were being
> used for upper nodes.  According to me behavior after the commit is
> consistent, for example, I think if you check the case of GatherMerge
> before this commit, you will still get the stats in the way it is
> after commit.

I understand. Maybe, this change should be mentioned in releases notes and/or
documentation?

Thanks,



signature.asc
Description: OpenPGP digital signature

Re: Explain buffers wrong counter with parallel plans

[Adrien Nayrat]

On 04/29/2018 05:11 PM, Amit Kapila wrote:
> I think you can try 'verbose' option, it will give per-worker stats.

Thanks! I didn't know verbose give per-woker stats!

-- 
Adrien




signature.asc
Description: OpenPGP digital signature

Re: XML/XPath issues: text/CDATA in XMLTABLE, XPath evaluated with wrong context

[Markus Winand]

Hi and thanks for your efforts.

> On 2018-04-26, at 21:18 , Alvaro Herrera  wrote:
> 
> Hello Markus,
> 
> Markus Winand wrote:
> 
>> * Patch 0001: Accept TEXT and CDATA nodes in XMLTABLE’s column_expression.
>> 
>>> Column_expressions that match TEXT or CDATA nodes must return
>>> the contents of the node itself, not the content of the
>>> non-existing childs (i.e. the empty string).
>> 
>> The following query returns a wrong result in master:
>> 
>> SELECT *
>>  FROM (VALUES ('text'::xml)
>> , (''::xml)
>>   ) d(x)
>>  CROSS JOIN LATERAL
>>XMLTABLE('/xml'
>> PASSING x
>> COLUMNS "node()" TEXT PATH 'node()'
>>) t
> 
>> The correct result can be obtained with patch 0001 applied:
>> 
>>   x| node()
>> +-
>> text| text
>>  | 
>> 
>> The patch adopts existing tests to guard against future regressions.
> 
> I remembered while reading this that Craig Ringer had commented that XML
> text sections can have multiple children: just put XML comments amidst
> the text.

As per my understanding of XML, this is a sibling—not a child—of two text nodes.

> To test this, I added a comment in one of the text values, in
> the regression database:
> 
> update xmldata set data = regexp_replace(data::text, 'HongKong', 'HongKong')::xml;
> 
> With the modified data, the new query in the regression tests fails:
> 
> SELECT  xmltable.*
>   FROM (SELECT data FROM xmldata) x,
>LATERAL XMLTABLE('/ROWS/ROW'
> PASSING data
> COLUMNS id int PATH '@id',
>  _id FOR ORDINALITY,
>  country_name text PATH 'COUNTRY_NAME/text()' 
> NOT NULL,
>  country_id text PATH 'COUNTRY_ID',
>  region_id int PATH 'REGION_ID',
>  size float PATH 'SIZE',
>  unit text PATH 'SIZE/@unit',
>  premier_name text PATH 'PREMIER_NAME' 
> DEFAULT 'not specified');
> 
> ERROR:  more than one value returned by column Path expression

I’ve tried matching “node()” against XML that includes a mixture of text and 
comment nodes in other database products. Two out of two failed with a
similar error message.

- ORA-19279: XPTY0004 - XQuery dynamic type mismatch: expected singleton 
sequence - got multi-item sequence
- SQLCODE=-16003, SQLSTATE=10507, SQLERRMC=( item(), item()+ )
  See: 
https://www.ibm.com/support/knowledgecenter/en/SSEPGG_11.1.0/com.ibm.db2.luw.messages.sql.doc/doc/msql16003n.html

So I digged through the standard to figure out what the standard-mandated
behaviour is.

The bottom line is that an error is the standard mandated behavior because
SQL uses a XPath/XQuery “cast" in the procedure. The XPath/XQuery spec
says “If the result of atomization is a sequence of more than one atomic value,
a type error is raised [err:XPTY0004]” (compare that to the ORA- above).
https://www.w3.org/TR/xpath-31/#id-cast

For reference, how I came there:
The XPath/XQuery cast is triggered for non XML types in
XMLCAST (SQL-14:2011 6.6 GR 4 h iv.) which is in turn triggered by
XMLTABLE (SQL-14:2011 7.1 SR 4 e ii 8).
[Note: I only have SQL-14:2011, thus no references to :2016]


> This seems really undesirable, so I looked into getting it fixed.  Turns
> out that this error is being thrown from inside the same function we're
> modifying, line 4559.  I didn't have a terribly high opinion of that
> ereport() when working on this feature to begin with, so now that it's
> proven to provide a bad experience, let's try removing it.  With that
> change (patch attached), the feature seems to work; I tried with this
> query, which seems to give the expected output (notice we have some
> columns of type xml, others of type text, with and without the text()
> function in the path):
> 
> SELECT  xmltable.*
>   FROM (SELECT data FROM xmldata) x,
>LATERAL XMLTABLE('/ROWS/ROW'
> PASSING data COLUMNS
>  country_name text PATH 'COUNTRY_NAME/text()' 
> NOT NULL,
>  country_name_xml xml PATH 
> 'COUNTRY_NAME/text()' NOT NULL,
>  country_name_n text PATH 'COUNTRY_NAME' NOT 
> NULL,
>  country_name_xml_n xml PATH 'COUNTRY_NAME' 
> NOT NULL);
>  country_name  │ country_name_xml │ country_name_n │  
> country_name_xml_n   
> ┼──┼┼───
> Australia  │ Australia│ Australia  │ 
> Australia
> China  │ China│ China  │ 
> China
> HongKong   │ HongKong │ HongKong   │ HongKong
> India  │ India│ 

Re: [RFC] Add an until-0 loop in psql

[Pierre Ducroquet]

On Monday, April 30, 2018 1:01:25 PM CEST Daniel Verite wrote:
>   Corey Huinker wrote:
> > As of v11, DO blocks can do transactions. I think this will meet your
> > needs.
> They do support COMMIT and ROLLBACK in the current
> development tree, but not VACUUM as in Pierre's example.
> 
> postgres=# \echo :SERVER_VERSION_NAME
> 11devel
> 
> postgres=# do ' begin vacuum; end ';
> ERROR:VACUUM cannot be executed from a function
> CONTEXT:  SQL statement "vacuum"
> PL/pgSQL function inline_code_block line 1 at SQL statement
> 
> 
> Best regards,

Indeed, vacuum is going to be the biggest offender here, sadly.
One could work around this of course (on top of my head, using notify to wake-
up another client that would launch the required vacuums…)
Being able to do transactions in DO blocks is a great new feature of v11 I was 
not aware of. But psql saw the addition of \if recently, so why not having 
loops in there too ? (Something better than this hack of course, it was just a 
10 minutes hack-sprint for a demo)

Regards

 Pierre

Re: [RFC] Add an until-0 loop in psql

[Daniel Verite]

Corey Huinker wrote:

> As of v11, DO blocks can do transactions. I think this will meet your needs.

They do support COMMIT and ROLLBACK in the current
development tree, but not VACUUM as in Pierre's example.

postgres=# \echo :SERVER_VERSION_NAME 
11devel

postgres=# do ' begin vacuum; end ';
ERROR:  VACUUM cannot be executed from a function
CONTEXT:  SQL statement "vacuum"
PL/pgSQL function inline_code_block line 1 at SQL statement


Best regards,
-- 
Daniel Vérité
PostgreSQL-powered mailer: http://www.manitou-mail.org
Twitter: @DanielVerite

Re: [HACKERS] Clock with Adaptive Replacement

[Andrey Borodin]

> 30 апр. 2018 г., в 0:48, Andres Freund  написал(а):
> 
> On 2018-04-25 11:31:12 +0500, Andrey Borodin wrote:
>> 
>> 1. Teaching BgWriter to used data from eviction strategy to aggressively 
>> flush data to disk (instead of ++next_to_clean )
>> 2. Implementing strategies as lock-free algorithms for freelist
>> These parts seem most important for benchmarking.
>> Also:
>> 3. Converting all rings to single buffer manager where possible
>> 4. Using O_DIRECT while writing data files
>> 5. Using aio and scheduling of writes
>> These parts are not necessary, but most challenging, while not impossible 
>> though.
> 
> These largely seem to be increasing the scope beyond reason...


I suspect that performance benefits can be not that big or even marginal, if we 
do not extend comprehensive eviction strategy with bgwriter fixes and O_DIRECT.
But, on the other hand, this suspicion is not based on any real fact. And if 
eviction strategy is actually good for anything it will show performance 
benefits on it's own.

> 30 апр. 2018 г., в 0:39, Юрий Соколов  написал(а):
> 
> (btw, why no one mention CLOCK+ in this thread).

I do not know what CLOCK+ is. Can you, please, send me a reference.

Best regards, Andrey Borodin.

Re: FOR EACH ROW triggers on partitioned tables

[Ashutosh Bapat]

On Fri, Mar 23, 2018 at 7:19 PM, Alvaro Herrera  wrote:
> Pushed.  Thanks for all the review.
>

https://www.postgresql.org/docs/devel/static/ddl-partitioning.html

section 5.10.2.3 still says
"Row triggers, if necessary, must be defined on individual partitions,
not the partitioned table."

Should that change?

Per commit 86f575948c773b0ec5b0f27066e37dd93a7f0a96, we are supporting
only AFTER row triggers. May be we should change the above line to
"Before row triggers, if necessary, must ".

-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

Re: Local partitioned indexes and pageinspect

[Michael Paquier]

On Sun, Apr 29, 2018 at 06:20:02PM -0700, Peter Geoghegan wrote:
> What about amcheck? I did change the example query in the docs to
> account for this, so anyone that generalizes from that won't have a
> problem, but it would be nice if it had a friendlier message. I didn't
> change amcheck to account for this myself because I thought it was
> possible that somebody else would want to use a more general solution.

Perhaps it would help if an errhint is added which is written as "This
relation is a %s", where %s is a relkind converted to a translatable
string describing the kind?  All those modules work on different objects
and have different goals and prospoectives, so it looks difficult to me
to come up with a sane API which is rather portable across modules.

The statu-quo is not completely bad either (aka no extra error
messages) as incorrect relation kind are still filtered out, perhaps the
docs don't emphasize enough that an index and a partitioned index are
two different things?
--
Michael


signature.asc
Description: PGP signature