Re: [HACKERS] patch: function xmltable

[Pavel Stehule]

Hi

2016-12-02 23:25 GMT+01:00 Alvaro Herrera :

> Here's version 17.  I have made significant changes here.
>
> 1. Restructure the execQual code.  Instead of a PG_TRY wrapper, I have
> split this code in three pieces; there's the main code with the PG_TRY
> wrappers and is mainly in charge of the builderCxt pointer.  In the
> previous coding there was a shim that examined builderCxt but was not
> responsible for setting it up, which was ugly.  The second part is the
> "initializer" which sets the row and column filters and does namespace
> processing.  The third part is the "FetchRow" logic.  It seems to me
> much cleaner this way.
>
> 2. rename the "builder" stuff to use the "routine" terminology.  This is
> in line with what we do for other function-pointer-filled structs, such
> as FdwRoutine, IndexAmRoutine etc.  I also cleaned up the names a bit
> more.
>
> 3. Added a magic number to the table builder context struct, so that we
> can barf appropriately.  This is in line with PgXmlErrorContext --
> mostly for future-proofing.  I didn't test this too hard.  Also, moved
> the XmlTableContext struct declaration nearer the top of the file, as is
> customary.  (We don't really need it that way, since the functions are
> all declared taking void *, but it seems cleaner to me anyway).
>
> 4. I added, edited, and fixed a large number of code comments.
>
> This is looking much better now, but it still needs at least the
> following changes.
>
> First, we need to fix is the per_rowset_memcxt thingy.  I think the way
> it's currently being used is rather ugly; it looks to me like the memory
> context does not belong into the XmlTableContext struct at all.
> Instead, the executor code should keep the memcxt pointer in a state
> struct of its own, and it should be the executor's responsibility to
> change to the appropriate context before calling the table builder
> functions.  In particular, this means that the table context can no
> longer be a void * pointer; it needs to be a struct that's defined by
> the executor (probably a primnodes.h one).  The void * pointer is
> stashed inside that struct.  Also, the "routine" pointer should not be
> part of the void * struct, but of the executor's struct.  So the
> execQual code can switch to the memory context, and destroy it
> appropriately.
>
> Second, we should make gram.y set a new "function type" value in the
> TableExpr it creates, so that the downstream code (transformTableExpr,
> ExecInitExpr, ruleutils.c) really knows that the given function is
> XmlTableExpr, instead of guessing just because it's the only implemented
> case.  Probably this "function type" is an enum (currently with a single
> value TableExprTypeXml or something like that) in primnodes.
>

It has sense - I was not sure about it - because currently it is only one
value, you mentioned it.


>
> Finally, there's the pending task of renaming and moving
> ExecTypeFromTableExpr to some better place.  Not sure that moving it
> back to nodeFuncs is a nice idea.  Looks to me like calling it from
> ExprTypmod is a rather ugly idea.
>

The code is related to prim nodes - it is used more times than in executor.

>
> Hmm, ruleutils ... not sure what to think of this one.
>

it is little bit more complex - but it is related to complexity of XMLTABLE


>
> The typedefs.list changes are just used to pgindent the affected code
> correctly.  It's not for commit.
>

The documentation is very precious. Nice

+/* XXX OK to do this?  looks a bit out of place ... */
+assign_record_type_typmod(typeInfo);

I am thinking it is ok. It is tupdesc without fixed typid, typname used in
returned value - you should to register this tupdesc in typcache.

Regards

Pavel



> --
> Álvaro Herrerahttps://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>

Re: [HACKERS] PSQL commands: \quit_if, \quit_unless

[Fabien COELHO]

Hello,


My guess is that something comparable to where pgbench is would be a
reasonable target --- not least because I think we should strive to
reduce unnecessary differences between psql and pgbench metalanguages.

I'm not sure that I'm ready to propose that they should share the same
expression engine, but perhaps it's not a totally wacky idea.


I'm trying to summarize a proposal for a conditional structure:

 - existing psql ":"-variables can be used, allowing anything from SQL
  (eg querying about available objects, features, extensions,
   current settings...)

 - target psql conditional syntax could be:

\if 
  ...
\elif <...>
  ...
\else
  ...
\endif

 - possible incremental implemention steps on this path:

  (1) minimal condition and expression, compatible with
  a possible future full-blown expression syntax

 \if :variable
 \if not :variable -- maybe \if ! :variable
   ...
 \endif

  (2) add "\else"

  (3) add "\elif ..." (or maybe "\elsif ..."?)

  (4) add greater but limited expressions, compatible with a full blown
  expression syntax (eg \if :var/const  :var/const)

  (5) add full-blown  support for \if, which suggest that
  it would also be available for \set


Does this looks okay, or does it need to be amended?

A few comments:

Given the experience with pgbench and the psql context, I do not think 
that it would really need to go beyond step 2 above, but I agree that I 
may be wrong and it is best to be prepared for that from the start. Given 
the complexity and effort involved with (5), it seems wise to wait for a 
clearer motivation with actual use-cases before going that far.


In the benchmarking context, the point is to test performance for a 
client-server scenario, in which case the client has to do some things, 
thus needs miminal computation capabilities which were available early in 
pgbench (\setrandom, \set with one arithmetic operation...) because they 
were necessary. Probably \if ... would make sense in pgbench, so I'll 
think about it.


In psql interactive context, conditions and expressions do not make sense 
as the user typing the command knows what they want, thus will do 
evaluations in their head to avoid typing stuff...


In psql scripting context, conditions are likely to be about what to do 
with the database, and what I understand of the use-case which started 
this discussion is that it is about versions, settings, available objects, 
typically "if this is already installed, skip this part" or "if version 
beyond YYY, cannot install because of missing features" when installing 
and initializing an application. For this purpose, ISTM that the query is 
necessarily performed server-side, thus the actual need for a full-blown 
client-side expression is limited or void, although as already said being 
prepared is a good thing.


--
Fabien.


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

Re: [HACKERS] Hash Indexes

[Amit Kapila]

On Sat, Dec 3, 2016 at 12:13 AM, Robert Haas  wrote:
> On Fri, Dec 2, 2016 at 1:54 AM, Amit Kapila  wrote:
>>>  I want to split when the average bucket
>>> contains 10 pages worth of tuples.
>>
>> oh, I think what you mean to say is hack the code to bump fill factor
>> and then test it.  I was confused that how can user can do that from
>> SQL command.
>
> Yes, that's why I said "hacking the fill factor up to 1000" when I
> originally mentioned it.
>
> Actually, for hash indexes, there's no reason why we couldn't allow
> fillfactor settings greater than 100, and it might be useful.
>

Yeah, I agree with that, but as of now, it might be tricky to support
the different range of fill factor for one of the indexes.  Another
idea could be to have an additional storage parameter like
split_bucket_length or something like that for hash indexes which
indicate that split will occur after the average bucket contains
"split_bucket_length * page" worth of tuples.  We do have additional
storage parameters for other types of indexes, so having one for the
hash index should not be a problem.

I think this is important because split immediately increases the hash
index space by approximately 2 times.  We might want to change that
algorithm someday, but the above idea will prevent that in many cases.

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


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

Re: [HACKERS] Parallel execution and prepared statements

[Amit Kapila]

On Fri, Dec 2, 2016 at 3:31 PM, Tobias Bussmann  wrote:
>
>> On Thu, Dec 1, 2016 at 9:40 PM, Robert Haas  wrote:
>>>
>>> OK, then my vote is to do it that way for now.
>
> Thanks for your opinion. That's fine with me.
>
>> Am 02.12.2016 um 07:22 schrieb Amit Kapila :
>> Done that way in attached patch.
>
> Did a quick review:
>

Thanks for the review.

> You should however include a sentence in the documentation on that parallel 
> plan w/o workers corner-case behaviour. Feel free to take that from my patch 
> or phase a better wording.
>

I have taken it from your patch.

> And again my question regarding back patching to 9.6:
> - 9.6 is currently broken as Laurenz showed in [1]
> - 9.6 does not have documented that SQL PREPARE prepared statements cannot 
> not use parallel query
>
> The former could be fixed by back patching the full patch which would void 
> the latter.
>

I think if we don't see any impact then we should backpatch this
patch. However, if we decide to go some other way, then I can provide
a separate patch for back branches.  BTW, what is your opinion?


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


prepared_stmt_parallel_query_v4.patch
Description: Binary data

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

Re: [HACKERS] [sqlsmith] Failed assertion in _hash_splitbucket_guts

[Amit Kapila]

On Sat, Dec 3, 2016 at 6:58 AM, Amit Kapila  wrote:
> On Sat, Dec 3, 2016 at 2:06 AM, Andreas Seltenreich  
> wrote:
>> Hi,
>>
>> the new hash index code on 11003eb failed an assertion yesterday:
>>
>> TRAP: FailedAssertion("!(bucket == obucket)", File: "hashpage.c", Line: 
>> 1037)
>
> _hash_expandtable(Relation rel, Buffer metabuf)
> {
> ..
> if (H_NEEDS_SPLIT_CLEANUP(oopaque))
> {
> /* Release the metapage lock. */
> _hash_chgbufaccess(rel, metabuf, HASH_READ, HASH_NOLOCK);
>
> hashbucketcleanup(rel, old_bucket, buf_oblkno, start_oblkno, NULL,
>   metap->hashm_maxbucket, metap->hashm_highmask,
>   metap->hashm_lowmask, NULL,
>   NULL, true, NULL, NULL);
> ..
> }
>
> Here we shouldn't be accessing meta page after releasing the lock as
> concurrent activity can change these values.  This can be fixed by
> storing these values in local variables before releasing the lock and
> passing local variables in hashbucketcleanup().  I will send patch
> shortly.
>

Please find attached patch to fix above code.  Now, if this is the
reason of the problem you are seeing, it won't fix your existing
database as it already contains some tuples in the wrong bucket.  Can
you please re-run the test to see if you can reproduce the problem?


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


fix_hash_bucketsplit_sqlsmith_v1.patch
Description: Binary data

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

Re: [HACKERS] [COMMITTERS] pgsql: Add max_parallel_workers GUC.

[Robert Haas]

On Dec 2, 2016, at 5:45 PM, Tom Lane  wrote:
> Jim Nasby  writes:
>>> On 12/2/16 2:34 PM, Robert Haas wrote:
>>> Signs point to "no".  It seemed like a good idea to leave some daylight 
>>> between max_parallel_workers and max_worker_processes, but evidently this 
>>> wasn't the way to get there. Or else we should just give up on that thought.
> 
>> Could the defaults be scaled based on max_connections, with a max on the 
>> default?
> 
> Might work.  We've had very bad luck with GUC variables with
> interdependent defaults, but maybe the user-visible knob could be a
> percentage of max_connections or something like that.

Seems like overkill. Let's just reduce the values a bit.

...Robert

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

Re: [HACKERS] Tackling JsonPath support

[Christian Convey]

On Fri, Dec 2, 2016 at 1:32 PM, Nico Williams  wrote:
...
On Fri, Dec 02, 2016 at 08:53:33AM -0500, Robert Haas wrote:

> > On Tue, Nov 29, 2016 at 11:50 AM, Christian Convey
> >  wrote:
> > > I think I can satisfy (3) with a PG extension which provides a
> function that
> > > approximately implements JSONPath.  My short-term plans are to submit
> such a
> > > patch.
> >
> > FWIW, I think that's a fine plan.  I don't really know whether
> > JSONPath is the right standard to pick for the task of extracting bits
>
> It's not even a standard.  Are there particular proposals that the ANSI
> SQL working group is considering?
>

​Hi Nico, it seems to be something in the works with the standards
committee.  We were discussing it earlier in the thread: [1]

Kind regards,
Christian

[1] http://www.mail-archive.com/pgsql-hackers@postgresql.org/msg297732.html​

Re: [HACKERS] [sqlsmith] Failed assertion in _hash_splitbucket_guts

[Amit Kapila]

On Sat, Dec 3, 2016 at 2:06 AM, Andreas Seltenreich  wrote:
> Hi,
>
> the new hash index code on 11003eb failed an assertion yesterday:
>
> TRAP: FailedAssertion("!(bucket == obucket)", File: "hashpage.c", Line: 
> 1037)
>

This can happen if we start new split before completing the previous
split of a bucket or if there is still any remaining tuples present in
the bucket being from the previous split.  I see a problem in below
code:

_hash_expandtable(Relation rel, Buffer metabuf)
{
..
if (H_NEEDS_SPLIT_CLEANUP(oopaque))
{
/* Release the metapage lock. */
_hash_chgbufaccess(rel, metabuf, HASH_READ, HASH_NOLOCK);

hashbucketcleanup(rel, old_bucket, buf_oblkno, start_oblkno, NULL,
  metap->hashm_maxbucket, metap->hashm_highmask,
  metap->hashm_lowmask, NULL,
  NULL, true, NULL, NULL);
..
}

Here we shouldn't be accessing meta page after releasing the lock as
concurrent activity can change these values.  This can be fixed by
storing these values in local variables before releasing the lock and
passing local variables in hashbucketcleanup().  I will send patch
shortly.  However, I wanted to verify that this is the reason why you
are seeing the problem.  I could not connect to the database provided
by you.

> Statement was
>
> update public.hash_i4_heap set seqno = public.hash_i4_heap.random;
>
> It can be reproduced with the data directory (Debian stretch amd64) I've
> put here:
>
> http://ansel.ydns.eu/~andreas/_hash_splitbucket_guts.tar.xz (12 MB)
>
> Backtrace below.  The cluster hasn't suffered any crashes before this
> incident.
>

How should I connect to this database?  If I use the user fdw
mentioned in pg_hba.conf (changed authentication method to trust in
pg_hba.conf), it says the user doesn't exist.  Can you create a user
in the database which I can use?

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


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

Re: [HACKERS] PSQL commands: \quit_if, \quit_unless

[Jim Nasby]

On 12/2/16 9:24 AM, Robert Haas wrote:

On Fri, Dec 2, 2016 at 11:12 AM, Corey Huinker  wrote:

In order for me to understand how high the bar has been set, can you
(Robert/Tom mostly, but I welcome any responses) explain what you mean by
"full-blown expression language"? What constructs must it include, etc?


I mostly mean it should be based on flex and bison, not "this is so
simple I can just hand-roll it".  I don't think it has much chance of
staying that simple.


It might be worth looking at other simplistic languages for guidance. 
bash comes to mind, though there's GPL issues there. csh/tcsh don't have 
those problems, and perhaps some of their grammar could be stolen.


I find it interesting that this is kind of the opposite problem that 
most pl's face: how to fit the database access paradigm into the 
language with the minimal amount of extra effort for users.


http://cvsweb.netbsd.org/bsdweb.cgi/src/bin/csh/?only_with_tag=MAIN
https://github.com/tcsh-org/tcsh
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)


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

Re: [HACKERS] [COMMITTERS] pgsql: Add max_parallel_workers GUC.

[Tom Lane]

Jim Nasby  writes:
> On 12/2/16 2:34 PM, Robert Haas wrote:
>> Signs point to "no".  It seemed like a good idea to leave some daylight 
>> between max_parallel_workers and max_worker_processes, but evidently this 
>> wasn't the way to get there. Or else we should just give up on that thought.

> Could the defaults be scaled based on max_connections, with a max on the 
> default?

Might work.  We've had very bad luck with GUC variables with
interdependent defaults, but maybe the user-visible knob could be a
percentage of max_connections or something like that.

regards, tom lane


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

Re: [HACKERS] [COMMITTERS] pgsql: Add max_parallel_workers GUC.

[Jim Nasby]

On 12/2/16 2:34 PM, Robert Haas wrote:

Signs point to "no".  It seemed like a good idea to leave some daylight between 
max_parallel_workers and max_worker_processes, but evidently this wasn't the way to get 
there. Or else we should just give up on that thought.


Could the defaults be scaled based on max_connections, with a max on the 
default?

--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)


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

Re: [HACKERS] [COMMITTERS] pgsql: Add max_parallel_workers GUC.

[Robert Haas]

On Dec 2, 2016, at 4:07 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> Add max_parallel_workers GUC.
>> Increase the default value of the existing max_worker_processes GUC
>> from 8 to 16, and add a new max_parallel_workers GUC with a maximum
>> of 8.
> 
> This broke buildfarm members coypu and sidewinder.  It appears the reason
> is that those machines can only get up to 30 server processes, cf this
> pre-failure initdb trace:
> 
> http://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=coypu=2016-12-02%2006%3A30%3A49=initdb-C
> 
> creating directory data-C ... ok
> creating subdirectories ... ok
> selecting default max_connections ... 30
> selecting default shared_buffers ... 128MB
> selecting dynamic shared memory implementation ... sysv
> creating configuration files ... ok
> running bootstrap script ... ok
> performing post-bootstrap initialization ... ok
> syncing data to disk ... ok
> 
> So you've reduced their available number of regular backends to less than
> 20, which is why their tests are now dotted with
> 
> ! psql: FATAL:  sorry, too many clients already
> 
> There may well be other machines with similar issues; we won't know until
> today's other breakage clears.
> 
> We could ask the owners of these machines to reduce the test parallelism
> via the MAX_CONNECTIONS makefile variable, but I wonder whether this
> increase was well thought out in the first place.

Signs point to "no".  It seemed like a good idea to leave some daylight between 
max_parallel_workers and max_worker_processes, but evidently this wasn't the 
way to get there. Or else we should just give up on that thought.

...Robert

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

Re: [HACKERS] HaveNFreeProcs() iterates through entire freeProcs list

[Jim Nasby]

On 12/2/16 12:52 PM, Tom Lane wrote:

I think you misread it.  Note the "n > 0" part of the while condition.


*facepalm*

Sorry for the noise...
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)


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

Re: [HACKERS] Tackling JsonPath support

[Nico Williams]

On Fri, Dec 02, 2016 at 08:53:33AM -0500, Robert Haas wrote:
> On Tue, Nov 29, 2016 at 11:50 AM, Christian Convey
>  wrote:
> > I think I can satisfy (3) with a PG extension which provides a function that
> > approximately implements JSONPath.  My short-term plans are to submit such a
> > patch.
> 
> FWIW, I think that's a fine plan.  I don't really know whether
> JSONPath is the right standard to pick for the task of extracting bits

It's not even a standard.  Are there particular proposals that the ANSI
SQL working group is considering?

> of JSON from other bits of JSON, but I think there's some value in
> picking something is simple enough that we can implement it in our own
> code and not have to rely on a third-party library.  Of course, if
> somebody feels like adding a configure option for --with-jq and

Sure.  My main concern is that I don't want to have to parse/format JSON
around every such call.  I'd rather parsed JSON remain in an internal
form for as long as possible.

Speaking of which, you could use libjq's jv API and not support the jq
language itself.

> appropriate interfaces to integrate with JQ, we could consider that,
> too, but that imposes a packaging requirement that a home-grown
> implementation doesn't.  I'd want to hear more than one vote for such

What we do in Heimdal, OpenAFS, and other open source projects, some
times, is include a copy / git submodule / similar of some such external
dependencies.  Naturally it's not possible to do this for all external
dependencies, but it works well enough.  The jv API part of jq is small
and simple, and could be ripped out into a library that could be
included in PostgreSQL.

> a course of action before embracing it.  If JQ is a Turing-complete
> query language, integrating it might be quite difficult -- for

Even if it weren't!  (It is.)

Consider this expression using a builtin in jq:

  [range(4503599627370496)]

That is, an array of integers from 0 to 4503599627370495, inclusive.
That will "halt" given a very, very large computer and a lot of time.

(Because jq is Turning-complete, range() can be coded in jq itself, and
some variants of range() are.)

> example, we'd need a way to make sure that it does periodic
> CHECK_FOR_INTERRUPTS() calls, and that it doesn't leak resources or
> crash if those calls decide longjmp() away due to an ERROR -- and
> would we let people query database tables with it?  Would that be
> efficient?  I think it's fine to have more limited objectives than
> what a JQ implementation would apparently entail.

Agreed.  I think this means that we need either or both of a variant of
the C jq_next() function that takes either a timeout parameter, or a
jq_intr() function that can cause a running jq_next() to stop.

(Tolerating longjmp() is harder to do and I'd rather not.)

Other projects, like, say, nginx or similar where there is a per-client
or per-connection memory pool to limit memory footprint, might want
libjq to get an allocator hook, so that's another enhancement to
consider.  If that's something that PostgreSQL would need, please let me
know.

Nico
-- 


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

Re: [HACKERS] Performance improvement for joins where outer side is unique

[Tom Lane]

Robert Haas  writes:
> On Dec 2, 2016, at 7:47 AM, Haribabu Kommi  wrote:
>> Patch still applies fine to HEAD.
>> Moved to next CF with "ready for committer" status.

> Tom, are you picking this up?

Yeah, I apologize for not having gotten to it in this commitfest, but
it's definitely something I will look at.

regards, tom lane


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

Re: [HACKERS] Performance improvement for joins where outer side is unique

[Robert Haas]

On Dec 2, 2016, at 7:47 AM, Haribabu Kommi  wrote:
>> On Wed, Nov 2, 2016 at 1:21 PM, David Rowley  
>> wrote:
>> On 31 October 2016 at 18:37, David Rowley  
>> wrote:
>> > I've rebased the changes I made to address this back in April to current 
>> > master.
>> 
>> Please note that I went ahead and marked this as "Ready for
>> committer". It was previously marked as such in a previous commitfest.
>> The changes made since last version was based on feedback from Tom.
>> 
>> If anyone thinks this is not correct then please mark as "Ready for review".
> 
> Patch still applies fine to HEAD.
> Moved to next CF with "ready for committer" status.

Tom, are you picking this up?

...Robert

Re: [HACKERS] [COMMITTERS] pgsql: Add max_parallel_workers GUC.

[Tom Lane]

Robert Haas  writes:
> Add max_parallel_workers GUC.
> Increase the default value of the existing max_worker_processes GUC
> from 8 to 16, and add a new max_parallel_workers GUC with a maximum
> of 8.

This broke buildfarm members coypu and sidewinder.  It appears the reason
is that those machines can only get up to 30 server processes, cf this
pre-failure initdb trace:

http://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=coypu=2016-12-02%2006%3A30%3A49=initdb-C

creating directory data-C ... ok
creating subdirectories ... ok
selecting default max_connections ... 30
selecting default shared_buffers ... 128MB
selecting dynamic shared memory implementation ... sysv
creating configuration files ... ok
running bootstrap script ... ok
performing post-bootstrap initialization ... ok
syncing data to disk ... ok

So you've reduced their available number of regular backends to less than
20, which is why their tests are now dotted with

! psql: FATAL:  sorry, too many clients already

There may well be other machines with similar issues; we won't know until
today's other breakage clears.

We could ask the owners of these machines to reduce the test parallelism
via the MAX_CONNECTIONS makefile variable, but I wonder whether this
increase was well thought out in the first place.

regards, tom lane


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

Re: [HACKERS] HaveNFreeProcs() iterates through entire freeProcs list

[Tom Lane]

Jim Nasby  writes:
> Current HaveNFreeProcs() iterates through the entire freeProcs list 
> (while holding ProcStructLock) just to determine if there's a small 
> number (superuser_reserved_connections) of free slots available.

I think you misread it.  Note the "n > 0" part of the while condition.

regards, tom lane


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

Re: [HACKERS] Radix tree for character conversion

[Heikki Linnakangas]

On 12/02/2016 10:18 PM, Alvaro Herrera wrote:

Heikki Linnakangas wrote:

On 11/09/2016 10:38 AM, Kyotaro HORIGUCHI wrote:

Thanks. The attached patch contains the patch by perlcritic.

0001,2,3 are Heikki's patch that are not modified since it is
first proposed. It's a bit too big so I don't attach them to this
mail (again).

https://www.postgresql.org/message-id/08e7892a-d55c-eefe-76e6-7910bc8dd...@iki.fi


I've now pushed these preliminary patches, with the applicable fixes from
you and Daniel. The attached patch is now against git master.


Is this the Nov. 30th commit?  Because I don't see any other commits
from you.


Yes. Sorry for the confusion.

- Heikki



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

Re: [HACKERS] Dynamic shared memory areas

[Thomas Munro]

On Sat, Dec 3, 2016 at 9:02 AM, Robert Haas  wrote:
> On Fri, Dec 2, 2016 at 2:56 PM, Robert Haas  wrote:
>> On Fri, Dec 2, 2016 at 1:21 PM, Robert Haas  wrote:
>>> On Thu, Dec 1, 2016 at 6:33 AM, Thomas Munro
>>>  wrote:
 Please find attached dsa-v8.patch, and also a small test module for
 running random allocate/free exercises and dumping the internal
 allocator state.
>>>
>>> OK, I've committed the main patch.
>>
>> ...but the buildfarm isn't very happy about it.
>>
>> tern complains:
>>
>> In file included from dsa.c:58:0:
>> ../../../../src/include/utils/dsa.h:59:1: error: unknown type name
>> 'pg_atomic_uint64'
>>  typedef pg_atomic_uint64 dsa_pointer_atomic;
>>
>> ...but that code is only compiled if #if DSA_POINTER_SIZEOF == 4 fails
>> to be true.  And that should always be true unless
>> PG_HAVE_ATOMIC_U64_SUPPORT is defined.  So apparently tern claims to
>> PG_HAVE_ATOMIC_U64_SUPPORT but doesn't actually define
>> pg_atomic_uint64?  That doesn't seem right.
>
> No, that's not the problem.  Just a garden variety thinko in dsa.h.
> Will push a fix presently.

Here's a patch to provide the right format string for dsa_pointer to
printf-like functions, which clears a warning coming from dsa_dump (a
debugging function) on 32 bit systems.

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


fix-dsa-pointer-format.patch
Description: Binary data

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

[HACKERS] [sqlsmith] Failed assertion in _hash_splitbucket_guts

[Andreas Seltenreich]

Hi,

the new hash index code on 11003eb failed an assertion yesterday:

TRAP: FailedAssertion("!(bucket == obucket)", File: "hashpage.c", Line: 
1037)

Statement was

update public.hash_i4_heap set seqno = public.hash_i4_heap.random;

It can be reproduced with the data directory (Debian stretch amd64) I've
put here:

http://ansel.ydns.eu/~andreas/_hash_splitbucket_guts.tar.xz (12 MB)

Backtrace below.  The cluster hasn't suffered any crashes before this
incident.

regards,
Andreas

Core was generated by `postgres: smith regression [local] UPDATE
   '.
Program terminated with signal SIGABRT, Aborted.
(gdb) bt
#0  0x7f49c40cc198 in __GI_raise (sig=sig@entry=6) at 
../sysdeps/unix/sysv/linux/raise.c:54
#1  0x7f49c40cd61a in __GI_abort () at abort.c:89
#2  0x007f55c1 in ExceptionalCondition 
(conditionName=conditionName@entry=0x84f890 "!(bucket == obucket)", 
errorType=errorType@entry=0x83665d "FailedAssertion", 
fileName=fileName@entry=0x84f86a "hashpage.c", 
lineNumber=lineNumber@entry=1037) at assert.c:54
#3  0x004a3d41 in _hash_splitbucket_guts (rel=rel@entry=0x1251ff8, 
metabuf=metabuf@entry=1703, obucket=obucket@entry=37, 
nbucket=nbucket@entry=549, obuf=obuf@entry=3082, nbuf=nbuf@entry=1754, 
htab=0x0, maxbucket=549, highmask=1023, lowmask=511) at hashpage.c:1037
#4  0x004a5627 in _hash_splitbucket (lowmask=511, highmask=1023, 
maxbucket=549, nbuf=1754, obuf=3082, nbucket=549, obucket=37, metabuf=1703, 
rel=0x1251ff8) at hashpage.c:894
#5  _hash_expandtable (rel=0x1251ff8, metabuf=1703) at hashpage.c:768
#6  0x004a1f71 in _hash_doinsert (rel=rel@entry=0x1251ff8, 
itup=itup@entry=0x26dc830) at hashinsert.c:236
#7  0x004a01c3 in hashinsert (rel=0x1251ff8, values=, 
isnull=, ht_ctid=0x26dc6fc, heapRel=, 
checkUnique=) at hash.c:247
#8  0x005ded1b in ExecInsertIndexTuples (slot=slot@entry=0x26dbd10, 
tupleid=tupleid@entry=0x26dc6fc, estate=estate@entry=0x2530028, 
noDupErr=noDupErr@entry=0 '\000', specConflict=specConflict@entry=0x0, 
arbiterIndexes=arbiterIndexes@entry=0x0) at execIndexing.c:388
#9  0x005fddaa in ExecUpdate (tupleid=tupleid@entry=0x7ffcaa7c9e40, 
oldtuple=oldtuple@entry=0x0, slot=slot@entry=0x26dbd10, 
planSlot=planSlot@entry=0x26db278, epqstate=epqstate@entry=0x26dac98, 
estate=estate@entry=0x2530028, canSetTag=1 '\001') at nodeModifyTable.c:1030
#10 0x005fe49c in ExecModifyTable (node=node@entry=0x26dabf0) at 
nodeModifyTable.c:1516
#11 0x005e3a18 in ExecProcNode (node=node@entry=0x26dabf0) at 
execProcnode.c:396
#12 0x005dfabe in ExecutePlan (dest=0x1c2ecd0, direction=, numberTuples=0, sendTuples=, operation=CMD_UPDATE, 
use_parallel_mode=, planstate=0x26dabf0, estate=0x2530028) at 
execMain.c:1567
#13 standard_ExecutorRun (queryDesc=0x1c2ed68, direction=, 
count=0) at execMain.c:338
#14 0x00701b94 in ProcessQuery (plan=, 
sourceText=0xfff228 "update public.hash_i4_heap set \n  seqno = 
public.hash_i4_heap.random\nreturning \n  (select option_value from 
information_schema.foreign_server_options limit 1 offset 2)\n as c0", 
params=0x0, dest=0x1c2ecd0, completionTag=0x7ffcaa7ca020 "") at pquery.c:185
#15 0x00701e0b in PortalRunMulti (portal=portal@entry=0x25c52b0, 
isTopLevel=isTopLevel@entry=1 '\001', setHoldSnapshot=setHoldSnapshot@entry=1 
'\001', dest=dest@entry=0x1c2ecd0, altdest=0xca30e0 , 
completionTag=completionTag@entry=0x7ffcaa7ca020 "") at pquery.c:1299
#16 0x007020f9 in FillPortalStore (portal=portal@entry=0x25c52b0, 
isTopLevel=isTopLevel@entry=1 '\001') at pquery.c:1045
#17 0x00702bcd in PortalRun (portal=portal@entry=0x25c52b0, 
count=count@entry=9223372036854775807, isTopLevel=isTopLevel@entry=1 '\001', 
dest=dest@entry=0x199f248, altdest=altdest@entry=0x199f248, 
completionTag=completionTag@entry=0x7ffcaa7ca3d0 "") at pquery.c:782
#18 0x00700379 in exec_simple_query (query_string=0xfff228 "update 
public.hash_i4_heap set \n  seqno = public.hash_i4_heap.random\nreturning \n  
(select option_value from information_schema.foreign_server_options limit 1 
offset 2)\n as c0") at postgres.c:1094
#19 PostgresMain (argc=, argv=argv@entry=0xfad1d8, 
dbname=, username=) at postgres.c:4069
#20 0x0046d6c9 in BackendRun (port=0xfa8c60) at postmaster.c:4271
#21 BackendStartup (port=0xfa8c60) at postmaster.c:3945
#22 ServerLoop () at postmaster.c:1701
#23 0x00698ab9 in PostmasterMain (argc=argc@entry=4, 
argv=argv@entry=0xf765d0) at postmaster.c:1309
#24 0x0046e88d in main (argc=4, argv=0xf765d0) at main.c:228


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

[HACKERS] HaveNFreeProcs() iterates through entire freeProcs list

[Jim Nasby]

Current HaveNFreeProcs() iterates through the entire freeProcs list 
(while holding ProcStructLock) just to determine if there's a small 
number (superuser_reserved_connections) of free slots available. For the 
common case, presumably it'd be faster to put the n<=0 test inside the 
loop and return as soon as that's true, instead of waiting until the end?


BTW, the comment certainly doesn't seem accurate for the current code, 
since performance will be determined entirely by the number of procs on 
freeProcs...


/*
 * Check whether there are at least N free PGPROC objects.
 *
 * Note: this is designed on the assumption that N will generally be small.
 */
bool
HaveNFreeProcs(int n)
{
PGPROC *proc;

SpinLockAcquire(ProcStructLock);

proc = ProcGlobal->freeProcs;

while (n > 0 && proc != NULL)
{
proc = (PGPROC *) proc->links.next;
n--;
}

SpinLockRelease(ProcStructLock);

return (n <= 0);
}
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)


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

Re: [HACKERS] Radix tree for character conversion

[Alvaro Herrera]

Heikki Linnakangas wrote:
> On 11/09/2016 10:38 AM, Kyotaro HORIGUCHI wrote:
> > Thanks. The attached patch contains the patch by perlcritic.
> > 
> > 0001,2,3 are Heikki's patch that are not modified since it is
> > first proposed. It's a bit too big so I don't attach them to this
> > mail (again).
> > 
> > https://www.postgresql.org/message-id/08e7892a-d55c-eefe-76e6-7910bc8dd...@iki.fi
> 
> I've now pushed these preliminary patches, with the applicable fixes from
> you and Daniel. The attached patch is now against git master.

Is this the Nov. 30th commit?  Because I don't see any other commits
from you.

-- 
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


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

Re: [HACKERS] Radix tree for character conversion

[Heikki Linnakangas]

On 11/09/2016 10:38 AM, Kyotaro HORIGUCHI wrote:

Thanks. The attached patch contains the patch by perlcritic.

0001,2,3 are Heikki's patch that are not modified since it is
first proposed. It's a bit too big so I don't attach them to this
mail (again).

https://www.postgresql.org/message-id/08e7892a-d55c-eefe-76e6-7910bc8dd...@iki.fi


I've now pushed these preliminary patches, with the applicable fixes 
from you and Daniel. The attached patch is now against git master.



0004 is radix-tree stuff, applies on top of the three patches
above.


I've spent the last couple of days reviewing this. While trying to 
understand how it works, I ended up dismantling, rewriting, and putting 
back together most of the added perl code. Attached is a new version, 
with more straightforward logic, making it more understandable. I find 
it more understandable, anyway, I hope it's not only because I wrote it 
myself :-). Let me know what you think.


In particular, I found the explanations of flat and segmented tables 
really hard to understand. So in this version, the radix trees for a 
conversion are stored completely in one large array. Leaf and 
intermediate levels are all in the same array. When reading this 
version, please note that I'm not sure if I mean the same thing with 
"segment" that you did in your version.


I moved the "lower" and "upper" values in the structs. Also, there are 
now also separate "lower" and "upper" values for the leaf levels of the 
trees, for 1- 2-, 3- and 4-byte inputs. This made a huge difference to 
the size of gb18030_to_utf8_radix.map, in particular: the source file 
shrank from about 2 MB to 1/2 MB. In that conversion, the valid range 
for the last byte of 2-byte inputs is 0x40-0xfe, and the valid range for 
the last byte of 4-byte inputs is 0x30-0x39. With the old patch version, 
the "chars" range was therefore 0x30-0xfe, to cover both of those, and 
most of the array was filled with zeros. With this new patch version, we 
store separate ranges for those, and can leave out most of the zeros.


There's a segment full of zeros at the beginning of each conversion 
array now. The purpose of that is that when traversing the radix tree, 
you don't need to check each intermediate value for 0. If you follow a 0 
offset, it simply points to the dummy all-zeros segments in the 
beginning. Seemed like a good idea to shave some cycles, although I'm 
not sure if it made much difference in reality.


I optimized pg_mb_radix_conv() a bit, too. We could do more. For 
example, I think it would save some cycles to have specialized versions 
of UtfToLocal and LocalToUtf, moving the tests for whether a combined 
character map and/or conversion callback is used, out of the loop. They 
feel a bit ugly too, in their current form...


I need a break now, but I'll try to pick this up again some time next 
week. Meanwhile, please have a look and tell me what you think.


- Heikki

diff --git a/src/backend/utils/mb/Unicode/Makefile b/src/backend/utils/mb/Unicode/Makefile
index ea21f4a..f184f65 100644
--- a/src/backend/utils/mb/Unicode/Makefile
+++ b/src/backend/utils/mb/Unicode/Makefile
@@ -40,23 +40,30 @@ WINMAPS = win866_to_utf8.map utf8_to_win866.map \
 
 GENERICMAPS = $(ISO8859MAPS) $(WINMAPS) \
 	gbk_to_utf8.map utf8_to_gbk.map \
-	koi8r_to_utf8.map utf8_to_koi8r.map
+	koi8r_to_utf8.map utf8_to_koi8r.map \
+	koi8u_to_utf8.map utf8_to_koi8u.map
 
 SPECIALMAPS = euc_cn_to_utf8.map utf8_to_euc_cn.map \
 	euc_jp_to_utf8.map utf8_to_euc_jp.map \
+	euc_jis_2004_to_utf8.map utf8_to_euc_jis_2004.map \
 	euc_kr_to_utf8.map utf8_to_euc_kr.map \
 	euc_tw_to_utf8.map utf8_to_euc_tw.map \
 	sjis_to_utf8.map utf8_to_sjis.map \
+	shift_jis_2004_to_utf8.map utf8_to_shift_jis_2004.map \
 	gb18030_to_utf8.map utf8_to_gb18030.map \
 	big5_to_utf8.map utf8_to_big5.map \
 	johab_to_utf8.map utf8_to_johab.map \
 	uhc_to_utf8.map utf8_to_uhc.map \
-	euc_jis_2004_to_utf8.map euc_jis_2004_to_utf8_combined.map \
+
+COMBINEDMAPS = euc_jis_2004_to_utf8.map euc_jis_2004_to_utf8_combined.map \
 	utf8_to_euc_jis_2004.map utf8_to_euc_jis_2004_combined.map \
 	shift_jis_2004_to_utf8.map shift_jis_2004_to_utf8_combined.map \
 	utf8_to_shift_jis_2004.map utf8_to_shift_jis_2004_combined.map
 
-MAPS = $(GENERICMAPS) $(SPECIALMAPS)
+MAPS = $(GENERICMAPS) $(SPECIALMAPS) $(COMBINEDMAPS)
+
+RADIXGENERICMAPS = $(subst .map,_radix.map,$(GENERICMAPS))
+RADIXMAPS = $(subst .map,_radix.map,$(GENERICMAPS) $(SPECIALMAPS))
 
 ISO8859TEXTS = 8859-2.TXT 8859-3.TXT 8859-4.TXT 8859-5.TXT \
 	8859-6.TXT 8859-7.TXT 8859-8.TXT 8859-9.TXT \
@@ -68,53 +75,76 @@ WINTEXTS = CP866.TXT CP874.TXT CP936.TXT \
 	CP1252.TXT CP1253.TXT CP1254.TXT CP1255.TXT \
 	CP1256.TXT CP1257.TXT CP1258.TXT
 
+SPECIALTEXTS = BIG5.TXT CNS11643.TXT \
+	CP932.TXT CP950.TXT \
+	JIS0201.TXT JIS0208.TXT JIS0212.TXT SHIFTJIS.TXT \
+	JOHAB.TXT KSX1001.TXT windows-949-2000.xml \
+	euc-jis-2004-std.txt sjis-0213-2004-std.txt \
+	gb-18030-2000.xml
+
 GENERICTEXTS = $(ISO8859TEXTS) 

Re: [HACKERS] Wrong order of tests in findDependentObjects()

[Tom Lane]

I wrote:
> Jim Nasby  writes:
>> On 12/1/16 1:09 PM, Tom Lane wrote:
>>> I think that the patch I wrote is good cleanup, so I'm still inclined
>>> to apply it in HEAD, but I no longer think it's fixing any case that's
>>> significant in the field.  I wonder if you have a counterexample?

>> No; I'm sure I've run into this because of a temp object other than a 
>> table (probably a function, though possibly something else).

> Makes sense.  The fact that we protect against this for temp tables and
> views would make it all the more surprising when you get bit by some
> less-common object type.

It occurred to me that the hack installed in 08dd23cec, to not record
a pg_depend entry for a temp table, causes its own set of misbehaviors.
If you drop the extension later in the same session, the temp table isn't
automatically removed.  This would cause repeated drop/create cycles to
fail, which is kind of annoying since you might well do that during
extension development.  Even more annoying, if the temp table has another
sort of dependency on the extension --- say, it uses a data type defined
by the extension --- the DROP EXTENSION would fail unless you say CASCADE.

So I've pushed this patch with an addition to revert that hack.  I added
a regression test case showing that such usage behaves properly now.

regards, tom lane


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

Re: [HACKERS] Dynamic shared memory areas

[Robert Haas]

On Fri, Dec 2, 2016 at 2:56 PM, Robert Haas  wrote:
> On Fri, Dec 2, 2016 at 1:21 PM, Robert Haas  wrote:
>> On Thu, Dec 1, 2016 at 6:33 AM, Thomas Munro
>>  wrote:
>>> Please find attached dsa-v8.patch, and also a small test module for
>>> running random allocate/free exercises and dumping the internal
>>> allocator state.
>>
>> OK, I've committed the main patch.
>
> ...but the buildfarm isn't very happy about it.
>
> tern complains:
>
> In file included from dsa.c:58:0:
> ../../../../src/include/utils/dsa.h:59:1: error: unknown type name
> 'pg_atomic_uint64'
>  typedef pg_atomic_uint64 dsa_pointer_atomic;
>
> ...but that code is only compiled if #if DSA_POINTER_SIZEOF == 4 fails
> to be true.  And that should always be true unless
> PG_HAVE_ATOMIC_U64_SUPPORT is defined.  So apparently tern claims to
> PG_HAVE_ATOMIC_U64_SUPPORT but doesn't actually define
> pg_atomic_uint64?  That doesn't seem right.

No, that's not the problem.  Just a garden variety thinko in dsa.h.
Will push a fix presently.

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


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

Re: [HACKERS] s/xlog/wal/ in tools and function names?

[Euler Taveira]

On 01-12-2016 23:02, Michael Paquier wrote:
>>> Should we also:
>>>
>>> - rename pg_switch_xlog and friends to pg_switch_wal?
>>> - rename pg_recievexlog to pg_revievewal (and others in bin/)?
>>> - rename pg_xlogdump to pg_waldump?
>>
>> I think yes to all.
> 
+1.

> I was hesitant to propose that, but if there is a will do move
> everything... Documentation would point to different pages if the
> utilities are renamed, so that's not helpful when comparing features
> across major releases... We may want to keep those files with their
> historical names.
> 
It seems confusing if we rename the tool but not the documentation file
name. Let's put a blinking message in the release notes saying 'tool X
was renamed to tool Y'. The only thing we should do is add on each tool
page something like: pg_waldump (formerly called pg_xlogdump) ...

>>> - if we do rename, should we keep aliases for functions and symlinks for
>>> tools?
>>
>> I think no.
> 
+1. If the packager wants to do those aliases, it is up to him/her.

> =# select proname from pg_proc where proname ~ 'xlog';
>  proname
> -
>  pg_current_xlog_location
>  pg_current_xlog_insert_location
>  pg_current_xlog_flush_location
>  pg_xlogfile_name_offset
>  pg_xlogfile_name
>  pg_xlog_location_diff
>  pg_last_xlog_receive_location
>  pg_last_xlog_replay_location
>  pg_is_xlog_replay_paused
>  pg_switch_xlog
>  pg_xlog_replay_pause
>  pg_xlog_replay_resume
> (12 rows)
> 
In those cases, let's keep the functions as wrappers and undocumented.
In a few releases, we could remove them without breaking softwares that
rely on them.


-- 
   Euler Taveira   Timbira - http://www.timbira.com.br/
   PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento


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

Re: [HACKERS] Dynamic shared memory areas

[Robert Haas]

On Fri, Dec 2, 2016 at 1:21 PM, Robert Haas  wrote:
> On Thu, Dec 1, 2016 at 6:33 AM, Thomas Munro
>  wrote:
>> Please find attached dsa-v8.patch, and also a small test module for
>> running random allocate/free exercises and dumping the internal
>> allocator state.
>
> OK, I've committed the main patch.

...but the buildfarm isn't very happy about it.

tern complains:

In file included from dsa.c:58:0:
../../../../src/include/utils/dsa.h:59:1: error: unknown type name
'pg_atomic_uint64'
 typedef pg_atomic_uint64 dsa_pointer_atomic;

...but that code is only compiled if #if DSA_POINTER_SIZEOF == 4 fails
to be true.  And that should always be true unless
PG_HAVE_ATOMIC_U64_SUPPORT is defined.  So apparently tern claims to
PG_HAVE_ATOMIC_U64_SUPPORT but doesn't actually define
pg_atomic_uint64?  That doesn't seem right.

The failures on several other BF members appear to be similar.

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


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

Re: [HACKERS] Hash Indexes

[Robert Haas]

On Fri, Dec 2, 2016 at 1:54 AM, Amit Kapila  wrote:
>>  I want to split when the average bucket
>> contains 10 pages worth of tuples.
>
> oh, I think what you mean to say is hack the code to bump fill factor
> and then test it.  I was confused that how can user can do that from
> SQL command.

Yes, that's why I said "hacking the fill factor up to 1000" when I
originally mentioned it.

Actually, for hash indexes, there's no reason why we couldn't allow
fillfactor settings greater than 100, and it might be useful.
Possibly it should be the default.  Not 1000, certainly, but I'm not
sure that the current value of 75 is at all optimal.  The optimal
value might be 100 or 125 or 150 or something like that.

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


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

Re: [HACKERS] Dynamic shared memory areas

[Robert Haas]

On Thu, Dec 1, 2016 at 6:33 AM, Thomas Munro
 wrote:
> Please find attached dsa-v8.patch, and also a small test module for
> running random allocate/free exercises and dumping the internal
> allocator state.

OK, I've committed the main patch.  As far as test-dsa.patch, can we
tie that into make check-world so that committing it delivers some
buildfarm coverage for this code?  Of course, the test settings would
have to be fairly conservative given that some buildfarm machines have
very limited resources, but it still seems worth doing.  test_shm_mq
might provide some useful precedent.

Note that you don't need the prototype if you've already used
PG_FUNCTION_INFO_V1.

I'm not sure that using the same random seed every time is a good
idea.  Maybe you should provide a way to set the seed as part of
starting the test, or to not do that (pass NULL?) and then elog(LOG,
...) the seed that's chosen.  Then if the BF crashes, we can see what
seed was in use for that particular test.

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


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

Re: [HACKERS] Logical Replication WIP

[Peter Eisentraut]

On 11/20/16 1:02 PM, Petr Jelinek wrote:
> 0001:
> This is the reworked approach to temporary slots that I sent earlier.

Andres, you had expressed an interest in this.  Will you be able to
review it soon?

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


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

Re: [HACKERS] PSQL commands: \quit_if, \quit_unless

[Robert Haas]

On Fri, Dec 2, 2016 at 11:12 AM, Corey Huinker  wrote:
> In order for me to understand how high the bar has been set, can you
> (Robert/Tom mostly, but I welcome any responses) explain what you mean by
> "full-blown expression language"? What constructs must it include, etc?

I mostly mean it should be based on flex and bison, not "this is so
simple I can just hand-roll it".  I don't think it has much chance of
staying that simple.

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


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

Re: [HACKERS] PSQL commands: \quit_if, \quit_unless

[Tom Lane]

Corey Huinker  writes:
> In order for me to understand how high the bar has been set, can you
> (Robert/Tom mostly, but I welcome any responses) explain what you mean by
> "full-blown expression language"? What constructs must it include, etc?

My guess is that something comparable to where pgbench is would be a
reasonable target --- not least because I think we should strive to
reduce unnecessary differences between psql and pgbench metalanguages.

I'm not sure that I'm ready to propose that they should share the same
expression engine, but perhaps it's not a totally wacky idea.

regards, tom lane


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

Re: [HACKERS] patch: function xmltable

[Alvaro Herrera]

Hm, you omitted tableexpr.h from the v15 patch ...

-- 
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


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

Re: [HACKERS] PSQL commands: \quit_if, \quit_unless

[Corey Huinker]

>
>
> The other problem with not thinking about that general case is that
> people will keep on proposing little bitty features that nibble at
> the problem but may or may not be compatible with a general solution.
> To the extent that such patches get accepted, we'll be forced into
> either backwards-compatibility breakage or sub-optimal solutions when
> we do get to the point of wanting a general answer.  I'd much rather
> start with a generalized design and then implement it piece by piece.
>
> (This is more or less the same point as my nearby stand against localized
> hacking of backslash parsing rules.)
>
> regards, tom lane
>


In order for me to understand how high the bar has been set, can you
(Robert/Tom mostly, but I welcome any responses) explain what you mean by
"full-blown expression language"? What constructs must it include, etc?

Re: [HACKERS] UNDO and in-place update

[Robert Haas]

On Fri, Dec 2, 2016 at 5:01 AM, Alexander Korotkov
 wrote:
> Idea of storing just one visibility bit in index tuple is a subject of
> serious doubts for me.
>
> 1. When definitely-all-visible isn't set then we have to recheck during
> scanning heap, right?
> But our current recheck (i.e. rechecking scan quals) is not enough.  Imagine
> you have a range
> index scan (val >= 100 AND val < 200), and val field was updated from val =
> 50 and val = 60
> in some row.  Then you might have this row duplicated in the output.
> Removing index scan and
> sticking to only bitmap index scan doesn't seem to be fair.  Thus, we need
> to introduce another
> kind of recheck that heapTuple.indexKey = indexTuple.indexKey.

Yes.

> 2. Another question is how it could work while index key being intensively
> updated.  There is a risk
> that we would have to traverse same UNDO-chains multiple times.  In worst
> case, we would have
> to spend quadratic time of number of row versions since our snapshot taken.
> We might try to mitigate this
> by caching TID => heap tuple map for our snapshot.  But I don't like this
> approach.
> If we have large enough index scan, then we could either run out of cache or
> consume too much memory
> for that cache.

I agree with the concern, but I don't think that's necessarily the
only mitigation strategy.  The details of what goes into UNDO and what
goes into the TPD aren't really defined yet, and if we structure those
so that you can efficiently find out what happened to a particular TID
with only a bounded number of accesses, then it might not be too bad.
If you imagine having to walk a chain of 1000 UNDO entries once per
TID, and there are 100 TIDs on the page, that sounds pretty bad.  But
maybe we can design the UNDO format in such a way that you never
actually need to walk the entire chain, or only in extremely rare
corner cases.

It strikes me that there are three possibilities.  One is that we can
design the UNDO-based visibility reconstruction so that it is
blazingly fast.  In that case, the only benefit of putting the
visibility information in the index is that index-only scans will be
able to avoid touching the heap in some cases where they presently
cannot.  The second possibility is that despite our best efforts the
UNDO-based visibility reconstruction is doomed to be cripplingly slow.
In that case, even in "good" cases like sequential scans and bitmap
heap scans where we can process the entire page at once instead of
possibly having to make a separate visit per TID, we'll still be
painfully slow.  In that case, we might as well forget this project
altogether.  The third is that we're somewhere in the middle.  If
UNDO-based visibility reconstruction is only a minor annoyance in
sequential scan and bitmap-heap scan cases but, despite our best
efforts, becomes highly painful in index scan cases, then the case for
putting XIDs in the index suddenly becomes a lot more interesting in
my mind.

We may well end up in exactly that place, but I think it's a little
too early to decide yet.  I think we need to write a detailed design
for how UNDO-based visibility reconstruction would actually work, and
maybe even build a prototype to see how that performs, before we can
decide on this.  I kind of hope we don't need to do XIDs-in-the-index;
it sounds like a lot of work, and this project is bound to be
extremely difficult even without that additional complication.
However, if it becomes clear that it's the only way for a system like
this to perform acceptably, then it'll have to be done (unless we give
up on the whole thing).

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


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

Re: [HACKERS] Patch: Implement failover on libpq connect level.

[Robert Haas]

On Thu, Dec 1, 2016 at 5:00 AM, Mithun Cy  wrote:
> On Wed, Nov 30, 2016 at 7:14 AM, Mithun Cy 
> wrote:
>> Thanks, send query resets the errorMessage. Will fix same.
>> PQsendQuery()->PQsendQueryStart()->resetPQExpBuffer(>errorMessage);
>
> Please find the patch which fixes this bug. conn->errorMessage as per
> definition only stores current error message, a new member
> conn->savedMessage is introduced to save and restore error messages related
> to previous hosts which we failed to connect.

Couldn't this just be a variable in PQconnectPoll(), instead of adding
a new structure member?

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


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

Re: [HACKERS] GIN non-intrusive vacuum of posting tree

[Robert Haas]

On Wed, Nov 30, 2016 at 11:38 AM, Andrew Borodin  wrote:
> This scan acquires cleanup lock on root of scan (not necessarily root
> of posting tree). Cleanup lock ensures no inserts are inside subtree.
> Scan traverses subtree DF taking exclusive locks from left to right.
> For any page being deleted all leftmost pages were locked and unlocked
> before. New reads cannot enter subtree, all old readscans were
> excluded by lock\unlock. Thus there shall not be deadlocks with
> ginStepRight().
>
> We get lock on page being deleted, then on a left page.
> ginStepRight() takes lock on left page, than on right page. But it’s
> presence is excluded by cleanup lock and DFS scan with locks of upper
> and left parts of tree.
>
> Thank you for reading this. Concurrency bothers me a lot. If you see
> that anything is wrong or suspicious, please do not hesitate to post
> your thoughts.

I don't know much about GIN, but this seems like an interesting
improvement.  I hope somebody who knows more about GIN will step up to
review it in depth.

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


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

Re: [HACKERS] Improvements in psql hooks for variables

[Rahila Syed]

I applied and tested the patch on latest master branch.

Kindly consider following comments,

ParseVariableBool(const char *value, const char *name)
+ParseVariableBool(const char *value, const char *name, bool *valid)
 {
size_t  len;

+   if (valid)
+   *valid = true;


  psql_error("unrecognized value \"%s\" for \"%s\": boolean
expected\n",
+  value, name);
+   if (valid)
+   *valid = false;


Why do we need this? IMO, valid should be always set to true if the value
is parsed to be correct.
There should not be an option to the caller to not follow the behaviour of
setting valid to either true/false.
As it is in the current patch, all callers of ParseVariableBool follow the
behaviour of setting valid with either true/false.

In following examples, incorrect error message is begin displayed.
“ON_ERROR_ROLLBACK” is an enum and also
accepts value 'interactive' .  The error message says boolean expected.

postgres=# \set ON_ERROR_ROLLBACK eretere
unrecognized value "eretere" for "ON_ERROR_ROLLBACK": boolean expected
\set: error while setting variable

Similarly for ECHO_HIDDEN which is also an enum and accepts value 'no_exec'

  postgres=# \set ECHO_HIDDEN NULL
unrecognized value "NULL" for "ECHO_HIDDEN": boolean expected
\set: error while setting variable


+   boolnewval = ParseVariableBool(opt, "\\timing", );
+   if (success)
+   pset.timing = newval;
+   }
else
pset.timing = !pset.timing;
if (!pset.quiet)
@@ -2660,7 +2669,14 @@ do_pset(const char *param, const char *value,
printQueryOpt *popt, bool quiet)
if (value && pg_strcasecmp(value, "auto") == 0)
popt->topt.expanded = 2;
else if (value)
-   popt->topt.expanded = ParseVariableBool(value, param);
+   {
+   boolvalid;
+   boolnewval = ParseVariableBool(value, param, );
+   if (valid)

Should same variable names (success / valid) be used for consistency?

Thank you,
Rahila Syed

On Wed, Nov 23, 2016 at 5:47 PM, Daniel Verite 
wrote:

>I wrote:
>
> > So I went through the psql commands which don't fail on parse errors
> > for booleans
> > [...]
>
> Here's a v5 patch implementing the suggestions mentioned upthread:
> all meta-commands calling ParseVariableBool() now fail
> when the boolean argument can't be parsed successfully.
>
> Also includes a minor change to SetVariableAssignHook() that now
> returns the result of the hook it calls after installing it.
> It doesn't make any difference in psql behavior since callers
> of SetVariableAssignHook() ignore its return value, but it's
> more consistent with SetVariable().
>
> Best regards,
> --
> Daniel Vérité
> PostgreSQL-powered mailer: http://www.manitou-mail.org
> Twitter: @DanielVerite
>

Re: [HACKERS] pgsql: Add putenv support for msvcrt from Visual Studio 2013

[Robert Haas]

On Wed, Nov 30, 2016 at 2:56 AM, Michael Paquier
 wrote:
> On Tue, Nov 29, 2016 at 08:45:13PM +0100, Christian Ullrich wrote:
>> * Michael Paquier wrote:
>>
>> > On Wed, Nov 16, 2016 at 12:45 PM, Christian Ullrich
>> >  wrote:
>> >> I also did a debug build with 1+2+4 that came in at 84 μs/iteration.
>> >
>> > Moved to next CF. Christian, perhaps this patch should have an extra
>> > round of reviews?
>>
>> It is significantly different from the last version, so yes, of course.
>
> Patches 0001 (Cosmetic fixes), 0002 (Remove unnecessary CloseHandle)
> and 0003 (support for debug CRTs) look in good shape to me. 0004 (Fix
> load race) is 0002 from the previous set, and this change makes sense
> in itself.

0001 looks fine insofar as it makes things consistent regarding 0 vs.
NULL, but the whitespace changes will be reverted by pgindent.  (I
just tested.)

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


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

Re: [HACKERS] s/xlog/wal/ in tools and function names?

[Vladimir Rusinov]

On Fri, Dec 2, 2016 at 1:59 PM Michael Paquier 
wrote:

> On Fri, Dec 2, 2016 at 8:28 PM, Vladimir Rusinov 
> wrote:
> > I guess it would make sense to do all of it in 10.0.
> > I'm new here, so not very sure about process. How many commit fests
> could I
> > expect before 10.0 is out (I'm a bit busy at the moment)?
>
> There are two remaining: 2017-01 and 2017-03:
> https://commitfest.postgresql.org/


Sweet, that's plenty of time.
Consider me signed up, probably by 2017-02 or 2017-03. If somebody else
wants to send some patches in this field earlier, please let me know so we
don't duplicate the work.


> > I'd rather have html redirects set up (assuming they are technically
> > possible).
>
> That makes the most sense, and that would need some coordination with
> the main documentation website to map a new page with the ones from
> the previous versions.
>


smime.p7s
Description: S/MIME Cryptographic Signature

[HACKERS] Re: [DOCS] monitoring.sgml - clarify length of query text displayed in pg_stat_statements

[Robert Haas]

On Wed, Nov 30, 2016 at 8:45 PM, Ian Barwick
 wrote:
> Small doc patch to clarify how much of the query text is show in
> pg_stat_statements
> and a link to the relevant GUC.

This patch improves the pg_stat_activity documentation, not the
pg_stat_statements documentation, but it looks correct, so I have
committed it.

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


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

Re: [HACKERS] s/xlog/wal/ in tools and function names?

[Michael Paquier]

On Fri, Dec 2, 2016 at 8:28 PM, Vladimir Rusinov  wrote:
> I guess it would make sense to do all of it in 10.0.
> I'm new here, so not very sure about process. How many commit fests could I
> expect before 10.0 is out (I'm a bit busy at the moment)?

There are two remaining: 2017-01 and 2017-03: https://commitfest.postgresql.org/

> I'd rather have html redirects set up (assuming they are technically
> possible).

That makes the most sense, and that would need some coordination with
the main documentation website to map a new page with the ones from
the previous versions.
-- 
Michael


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

Re: [HACKERS] Tackling JsonPath support

[Robert Haas]

On Tue, Nov 29, 2016 at 11:50 AM, Christian Convey
 wrote:
> I think I can satisfy (3) with a PG extension which provides a function that
> approximately implements JSONPath.  My short-term plans are to submit such a
> patch.

FWIW, I think that's a fine plan.  I don't really know whether
JSONPath is the right standard to pick for the task of extracting bits
of JSON from other bits of JSON, but I think there's some value in
picking something is simple enough that we can implement it in our own
code and not have to rely on a third-party library.  Of course, if
somebody feels like adding a configure option for --with-jq and
appropriate interfaces to integrate with JQ, we could consider that,
too, but that imposes a packaging requirement that a home-grown
implementation doesn't.  I'd want to hear more than one vote for such
a course of action before embracing it.  If JQ is a Turing-complete
query language, integrating it might be quite difficult -- for
example, we'd need a way to make sure that it does periodic
CHECK_FOR_INTERRUPTS() calls, and that it doesn't leak resources or
crash if those calls decide longjmp() away due to an ERROR -- and
would we let people query database tables with it?  Would that be
efficient?  I think it's fine to have more limited objectives than
what a JQ implementation would apparently entail.

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


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

Re: [HACKERS] Write Ahead Logging for Hash Indexes

[Robert Haas]

On Thu, Dec 1, 2016 at 6:51 PM, Amit Kapila  wrote:
>> Thanks.  I am thinking that it might make sense to try to get the
>> "microvacuum support for hash index" and "cache hash index meta page"
>> patches committed before this one, because I'm guessing they are much
>> simpler than this one, and I think therefore that the review of those
>> patches can probably move fairly quickly.
>
> I think it makes sense to move "cache hash index meta page" first,
> however "microvacuum support for hash index" is based on WAL patch as
> the action in this patch (delete op) also needs to be logged.  One
> idea could be that we can try to split the patch so that WAL logging
> can be done as a separate patch, but I am not sure if it is worth.

The thing is, there's a fair amount locking stupidity in what just got
committed because of the requirement that the TID doesn't decrease
within a page.  I'd really like to get that fixed.

>>  Of course, ideally I can
>> also start reviewing this one in the meantime.  Does that make sense
>> to you?
>>
>
> You can start reviewing some of the operations like "Create Index",
> "Insert".  However, some changes are required because of change in
> locking strategy for Vacuum.  I am planning to work on rebasing it and
> making required changes in next week.

I'll review after that, since I have other things to review meanwhile.

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


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

[HACKERS] Re: 答复: [HACKERS] Re: [HACKERS] 答复: [HACKERS] postgres 1 个(共 2 个) can pg 9.6 vacuum freeze skip page on index?

[Robert Haas]

On Thu, Dec 1, 2016 at 9:54 PM, xu jian  wrote:
> Thanks every for your help. I am not familiar with the internal of the
> vacuum freeze, just curious if there is no row change on the table(in other
> words, all pages are frozen), why could index page have dead tuple?

It can't.   If the *entire* table is frozen, then I would think it
shouldn't be scanning the indexes.

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


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

Re: [HACKERS] Logical Replication WIP

[Alvaro Herrera]

Petr Jelinek wrote:
> On 02/12/16 02:55, Thomas Munro wrote:

> > Commit 597a87ccc9a6fa8af7f3cf280b1e24e41807d555 left some comments
> > behind that referred to the select() that it removed.  Maybe rewrite
> > like in the attached?
> 
> Agreed.

Thanks, pushed.


-- 
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


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

Re: [HACKERS] Performance improvement for joins where outer side is unique

[Haribabu Kommi]

On Wed, Nov 2, 2016 at 1:21 PM, David Rowley 
wrote:

> On 31 October 2016 at 18:37, David Rowley 
> wrote:
> > I've rebased the changes I made to address this back in April to current
> master.
>
> Please note that I went ahead and marked this as "Ready for
> committer". It was previously marked as such in a previous commitfest.
> The changes made since last version was based on feedback from Tom.
>
> If anyone thinks this is not correct then please mark as "Ready for
> review".
>

Patch still applies fine to HEAD.
Moved to next CF with "ready for committer" status.

Regards,
Hari Babu
Fujitsu Australia

Re: [HACKERS] Rename max_parallel_degree?

[Robert Haas]

On Thu, Dec 1, 2016 at 10:07 PM, Haribabu Kommi
 wrote:
> From the recent mails, it is not clear to me what is the status of this
> patch.
> moved to next CF with "needs review" state. Please feel free to update
> the status.

I have committed this patch.  And updated the status, too!

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


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

Re: [HACKERS] Logical Replication WIP

[Petr Jelinek]

On 02/12/16 02:55, Thomas Munro wrote:
> On Fri, Dec 2, 2016 at 2:32 PM, Peter Eisentraut
>  wrote:
>> On 11/30/16 8:06 PM, Petr Jelinek wrote:
>>> On 30/11/16 22:37, Peter Eisentraut wrote:
 I have taken the libpqwalreceiver refactoring patch and split it into
 two: one for the latch change, one for the API change.  I have done some
 mild editing.

 These two patches are now ready to commit in my mind.
>>
>>> Hi, looks good to me, do you plan to commit this soon or would you
>>> rather me to resubmit the patches rebased on top of this (and including
>>> this) first?
>>
>> committed those two
> 
> Commit 597a87ccc9a6fa8af7f3cf280b1e24e41807d555 left some comments
> behind that referred to the select() that it removed.  Maybe rewrite
> like in the attached?

Agreed.

> 
> I wonder if it would be worth creating and reusing a WaitEventSet here.
> 

I don't think it's worth the extra code given that this is rarely called
interface.

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


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

Re: [HACKERS] WIP: [[Parallel] Shared] Hash

[Haribabu Kommi]

On Thu, Nov 3, 2016 at 4:19 PM, Thomas Munro 
wrote:

> Obviously I'm actively working on developing and stabilising all this.
> Some of the things I'm working on are: work_mem accounting, batch
> increases, rescans and figuring out if the resource management for
> those BufFiles is going to work.  There are quite a lot of edge cases
> some of which I'm still figuring out, but I feel like this approach is
> workable.  At this stage I want to share what I'm doing to see if
> others have feedback, ideas, blood curdling screams of horror, etc.  I
> will have better patches and a set of test queries soon.  Thanks for
> reading.
>
>
This patch doesn't receive any review. Patch is not applying properly to
HEAD.
Moved to next CF with "waiting on author" status.

Regards,
Hari Babu
Fujitsu Australia

Re: [HACKERS] background sessions

[Haribabu Kommi]

On Tue, Nov 1, 2016 at 2:25 PM, Peter Eisentraut <
peter.eisentr...@2ndquadrant.com> wrote:

> Here is a patch for the background sessions C API and PL/Python support.
>  This was previously submitted as "autonomous transactions", which
> proved controversial, and there have been several suggestions for a new
> name.
>
> I have renamed everything, removed all the incomplete PL/pgSQL stuff,
> did some refinement on the PL/Python interfaces, added resource owner
> management so that you can preserve session handles across transactions.
>  That allows a pg_background-like behavior implemented in a PL function.
>  I have also added documentation, so reviewers could start there.
> Probably not quite all done yet, but I think it contains a lot of useful
> pieces that we could make into something nice.
>
>
Moved to next CF with "needs review" status.


Regards,
Hari Babu
Fujitsu Australia

Re: ParallelFinish-hook of FDW/CSP (Re: [HACKERS] Steps inside ExecEndGather)

[Haribabu Kommi]

On Tue, Nov 1, 2016 at 1:33 AM, Kouhei Kaigai  wrote:

> Hello,
>
> The attached patch implements the suggestion by Amit before.
>
> What I'm motivated is to collect extra run-time statistics specific
> to a particular ForeignScan/CustomScan, not only the standard
> Instrumentation; like DMA transfer rate or execution time of GPU
> kernels in my case.
>
> Per-node DSM toc is one of the best way to return run-time statistics
> to the master backend, because FDW/CSP can assign arbitrary length of
> the region according to its needs. It is quite easy to require.
> However, one problem is, the per-node DSM toc is already released when
> ExecEndNode() is called on the child node of Gather.
>
> This patch allows extensions to get control on the master backend's
> context when all the worker node gets finished but prior to release
> of the DSM segment. If FDW/CSP has its special statistics on the
> segment, it can move to the private memory area for EXPLAIN output
> or something other purpose.
>
> One design consideration is whether the hook shall be called from
> ExecParallelRetrieveInstrumentation() or ExecParallelFinish().
> The former is a function to retrieve the standard Instrumentation
> information, thus, it is valid only if EXPLAIN ANALYZE.
> On the other hands, if we put entrypoint at ExecParallelFinish(),
> extension can get control regardless of EXPLAIN ANALYZE, however,
> it also needs an extra planstate_tree_walker().
>
> Right now, we don't assume anything onto the requirement by FDW/CSP.
> It may want run-time statistics regardless of EXPLAIN ANALYZE, thus,
> hook shall be invoked always when Gather node confirmed termination
> of the worker processes.
>
>
Moved to next CF with "needs review" status.

Regards,
Hari Babu
Fujitsu Australia

Re: [HACKERS] Proposal: scan key push down to heap [WIP]

[Haribabu Kommi]

On Fri, Dec 2, 2016 at 8:41 AM, Andres Freund  wrote:

> On 2016-11-30 16:11:23 +0530, Dilip Kumar wrote:
> > On Tue, Nov 29, 2016 at 11:21 PM, Robert Haas 
> wrote:
> > > On Mon, Nov 28, 2016 at 11:17 PM, Dilip Kumar 
> wrote:
> > >> Actually we want to call slot_getattr instead heap_getattr, because of
> > >> problem mentioned by Andres upthread and we also saw in test results.
> > >
> > > Ah, right.
> > >
> > >> Should we make a copy of HeapKeyTest lets say ExecKeyTest and keep it
> > >> under executor ?
> > >
> > > Sure.
> >
> > I have worked on the idea you suggested upthread. POC patch is
> > attached.
>
> Hm. I'm more than a bit doubful about this approach. Shouldn't we just
> *always* do this as part of expression evaluation, instead of
> special-casing for seqscans?
>
> I.e. during planning recognize that an OpExpr can be evaluated as a
> scankey and then emit different qual evaluation instructions?  Because
> then the benefit can be everywhere, instead of just seqscans.
>
> I'll post my new expression evaluation stuff - which doesn't do this
> atm, but makes ExecQual faster in other ways - later this week.  If we
> could get the planner (or parse-analysis?) to set an OpExpr flag that
> signals that the expression can be evaluated as a scankey, that'd be
> easy.
>
>
Moved to next CF with "waiting on author" status.

Regards,
Hari Babu
Fujitsu Australia

Re: [HACKERS] Proposal : For Auto-Prewarm.

[Haribabu Kommi]

On Tue, Nov 29, 2016 at 4:26 PM, Mithun Cy 
wrote:

> Sorry I took some time on this please find latest patch with addressed
> review comments. Apart from fixes for comments I have introduced a new GUC
> variable for the pg_autoprewarm "buff_dump_interval". So now we dump the
> buffer pool metadata at every buff_dump_interval secs. Currently
> buff_dump_interval can be set only at startup time. I did not choose to do
> the dumping at checkpoint time, as it appeared these 2 things are not much
> related and keeping it independent would be nice for usage. Also overhead
> of any communication between them can be avoided.
>
> On Fri, Oct 28, 2016 at 1:45 AM, Jim Nasby 
> wrote:
>  > IMO it would be better to add this functionality to pg_prewarm instead
> of creating another contrib module. That would reduce confusion and reduce
>  > the amount of code necessary.
>
> I have merged pg_autoprewarm module into pg_prewarm, This is just the
> directory merge, Functionality merge is not possible pg_prewarm is just a
> utility function with specific signature to load a specific relation at
> runtime, where as pg_autoprewarm is a bgworker which dumps current buffer
> pool and load it during startup time.
>
> > Presumably the first 4 numbers will vary far less than blocknum, so it's
> probably worth reversing the order here (or at least put blocknum first).
>
> function sort_cmp_func is for qsort so orderly comparison is needed to say
> which is bigger or smaller, If we put blocknum first, we cannot decide same.
>
> > AFAICT this isn't necessary since palloc will error itself if it fails.
>
> Fixed.
>
> > Since there's no method to change DEFAULT_DUMP_FILENAME, I would call it
> what it is: DUMP_FILENAME.
>
> Fixed.
>
> > Also, maybe worth an assert to make sure there was enough room for the
> complete filename. That'd need to be a real check if this was configurable
> > anyway.
>
> I think if we make it configurable I think I can put that check.
>
>  > + if (!avoid_dumping)
>  > +   dump_now();
>  > Perhaps that check should be moved into dump_now() itself...
>
>  Fixed.
>
>
Moved to next CF with "needs review" status.

Regards,
Hari Babu
Fujitsu Australia

Re: [HACKERS] Supporting huge pages on Windows

[Haribabu Kommi]

On Tue, Oct 11, 2016 at 1:36 PM, Tsunakawa, Takayuki <
tsunakawa.ta...@jp.fujitsu.com> wrote:

>
>
Moved to next CF with "needs review" status.


Regards,
Hari Babu
Fujitsu Australia

Re: [HACKERS] Improvements in psql hooks for variables

[Haribabu Kommi]

On Wed, Nov 23, 2016 at 11:17 PM, Daniel Verite 
wrote:

>I wrote:
>
> > So I went through the psql commands which don't fail on parse errors
> > for booleans
> > [...]
>
> Here's a v5 patch implementing the suggestions mentioned upthread:
> all meta-commands calling ParseVariableBool() now fail
> when the boolean argument can't be parsed successfully.
>
> Also includes a minor change to SetVariableAssignHook() that now
> returns the result of the hook it calls after installing it.
> It doesn't make any difference in psql behavior since callers
> of SetVariableAssignHook() ignore its return value, but it's
> more consistent with SetVariable().


Moved to next CF with "needs review" status.

Regards,
Hari Babu
Fujitsu Australia

Re: [HACKERS] patch proposal

[Haribabu Kommi]

On Tue, Nov 22, 2016 at 10:17 PM, Haribabu Kommi 
wrote:

>
> Hi Abhijit,
>
> This is a gentle reminder.
>
> you assigned as reviewer to the current patch in the 11-2016 commitfest.
> But you haven't shared your review yet. Please share your review about
> the patch. This will help us in smoother operation of commitfest.
>
> Please Ignore if you already shared your review.
>

Moved to next CF with "needs review" status.

Regards,
Hari Babu
Fujitsu Australia

Re: [HACKERS] Dynamic shared memory areas

[Haribabu Kommi]

On Thu, Dec 1, 2016 at 10:33 PM, Thomas Munro  wrote:

>
> Please find attached dsa-v8.patch, and also a small test module for
> running random allocate/free exercises and dumping the internal
> allocator state.


Moved to next CF with "needs review" status.

Regards,
Hari Babu
Fujitsu Australia

Re: [HACKERS] WIP: multivariate statistics / proof of concept

[Haribabu Kommi]

On Tue, Nov 22, 2016 at 2:42 PM, Tomas Vondra 
wrote:

> On 11/21/2016 11:10 PM, Robert Haas wrote:
>
>> [ reviving an old multivariate statistics thread ]
>>
>> On Thu, Nov 13, 2014 at 6:31 AM, Simon Riggs 
>> wrote:
>>
>>> On 12 October 2014 23:00, Tomas Vondra  wrote:
>>>
>>> It however seems to be working sufficiently well at this point, enough
 to get some useful feedback. So here we go.

>>>
>>> This looks interesting and useful.
>>>
>>> What I'd like to check before a detailed review is that this has
>>> sufficient applicability to be useful.
>>>
>>> My understanding is that Q9 and Q18 of TPC-H have poor plans as a
>>> result of multi-column stats errors.
>>>
>>> Could you look at those queries and confirm that this patch can
>>> produce better plans for them?
>>>
>>
>> Tomas, did you ever do any testing in this area?  One of my
>> colleagues, Rafia Sabih, recently did some testing of TPC-H queries @
>> 20 GB.  Q18 actually doesn't complete at all right now because of an
>> issue with the new simplehash implementation.  I reported it to Andres
>> and he tracked it down, but hasn't posted the patch yet - see
>> http://archives.postgresql.org/message-id/20161115192802.jfb
>> ec5s6ougxw...@alap3.anarazel.de
>>
>> Of the remaining queries, the slowest are Q9 and Q20, and both of them
>> have serious estimation errors.  On Q9, things go wrong here:
>>
>>  ->  Merge Join
>> (cost=5225092.04..6595105.57 rows=154 width=47) (actual
>> time=103592.821..149335.010 rows=6503988 loops=1)
>>Merge Cond:
>> (partsupp.ps_partkey = lineitem.l_partkey)
>>Join Filter:
>> (lineitem.l_suppkey = partsupp.ps_suppkey)
>>Rows Removed by Join Filter:
>> 19511964
>>->  Index Scan using
>>
> > [snip]
>
>>
>> Rows Removed by Filter: 756627
>>
>> The estimate for the index scan on partsupp is essentially perfect,
>> and the lineitem-part join is off by about 3x.  However, the merge
>> join is off by about 4000x, which is real bad.
>>
>>
> The patch only deals with statistics on base relations, no joins, at this
> point. It's meant to be extended in that direction, so the syntax supports
> it, but at this point that's all. No joins.
>
> That being said, this estimate should be improved in 9.6, when you create
> a foreign key between the tables. In fact, that patch was exactly about Q9.
>
> This is how the join estimate looks on scale 1 without the FK between the
> two tables:
>
>   QUERY PLAN
> ---
>  Merge Join  (cost=19.19..700980.12 rows=2404 width=261)
>Merge Cond: ((lineitem.l_partkey = partsupp.ps_partkey) AND
> (lineitem.l_suppkey = partsupp.ps_suppkey))
>->  Index Scan using idx_lineitem_part_supp on lineitem
> (cost=0.43..605856.84 rows=6001117 width=117)
>->  Index Scan using partsupp_pkey on partsupp
> (cost=0.42..61141.76 rows=80 width=144)
> (4 rows)
>
>
> and with the foreign key:
>
>  QUERY PLAN
> ---
>  Merge Join  (cost=19.19..700980.12 rows=6001117 width=261)
>  (actual rows=6001215 loops=1)
>Merge Cond: ((lineitem.l_partkey = partsupp.ps_partkey) AND
> (lineitem.l_suppkey = partsupp.ps_suppkey))
>->  Index Scan using idx_lineitem_part_supp on lineitem
> (cost=0.43..605856.84 rows=6001117 width=117)
> (actual rows=6001215 loops=1)
>->  Index Scan using partsupp_pkey on partsupp
> (cost=0.42..61141.76 rows=80 width=144)
> (actual rows=6001672 loops=1)
>  Planning time: 3.840 ms
>  Execution time: 21987.913 ms
> (6 rows)
>
>
> On Q20, things go wrong here:
>>
> >
>
>> [snip]
>>
>> The estimate for the GroupAggregate feeding one side of the merge join
>> is quite accurate.  The estimate for the part-partsupp join on the
>> other side is off by 8x.  Then things get much worse: the estimate for
>> the merge join is off by 400x.
>>
>>
> Well, most of the estimation error comes from the join, but sadly the
> aggregate makes using the foreign keys impossible - at least in the current
> version. I don't know if it can be improved, somehow.
>
> I'm not really sure whether the multivariate statistics stuff will fix
>> this kind of case or not, but if it did it would be awesome.
>>
>>
> Join statistics are something I'd like to add eventually, but I don't see
> how it could happen in the first version. Also, the patch received no
> reviews this CF, and making it even larger is unlikely to make it more
> attractive.
>

Moved to next CF with "needs review" status.

Regards,
Hari Babu
Fujitsu Australia

Re: [HACKERS] DROP FUNCTION of multiple functions

[Haribabu Kommi]

On Fri, Dec 2, 2016 at 1:32 PM, Peter Eisentraut <
peter.eisentr...@2ndquadrant.com> wrote:

> On 11/23/16 5:04 PM, Tom Lane wrote:
> > I looked at this briefly.  I agree that 0001-0003 are simple cleanup of
> > the grammar and could be pushed without further ado.
>
> Done.
>
> > However, starting
> > with 0004 I begin to get queasy.  The plan seems to be that instead of
> > "objname" always being a List of strings, for functions (and then
> > aggregates and operators) it will be a one-element List of some new
> struct
> > that has then got a name list inside it.
>
> I think the original idea of representing an object by a list of name
> components plus optionally a list of arguments has accumulated too many
> warts and should be replaced.  For example: A Typename isn't a list, so
> it has to be packed into a List to be able to pass it around.  Some
> objects only have a single-component string as a name.  For a cast, we
> arbitrarily put the source type into a the name list and the target type
> into the argument list.  For an operator class on the other hand we
> create a cons of name and access method.  The pending logical
> replication patch has more such arbitrary examples.  This pattern has to
> be repeated consistently in gram.y for all cases where the object is
> referenced (ALTER EXTENSION, DROP, COMMENT, RENAME, SET SCHEMA, OWNER
> TO) and then consistently unpacked in objectaddress.c.
>
> I think it would be better to get rid of objargs and have objname be a
> general Node that can contain more specific node types so that there is
> some amount of type tracking.  FuncWithArgs would be one such type,
> Typename would be another, Value would be used for simple strings, and
> we could create some other ones, or stick with lcons for some simple
> cases.  But then we don't have to make stuff into one-item lists to just
> to satisfy the currently required List.
>
> That's the general idea.  But that's a rather big change that I would
> rather break down into smaller pieces.  I have a separate patch in
> progress for that, which I have attached here.  It breaks some
> regression tests in object_address.sql, which I haven't evaluated yet,
> but that's the idea.
>
> However, in these current patches I just wanted to take the first step
> to normalize the representation of functions so that actual
> functionality changes could be built in top of that.
>
> > It leads to code with random changes of data representation at seemingly
> > random places, like this bit from 0005:
> >
> > + if (stmt->removeType == OBJECT_AGGREGATE ||
> stmt->removeType == OBJECT_FUNCTION)
> > + objname = list_make1(objname);
>
> Yeah, the reason for that is that when we want to store something as
> objname, it needs to be a list, and the convention is to wrap non-lists
> into a single-member list.  So then objectaddress.c is coded to linitial
> such lists when working on object types such as functions or types.
> RemoveObjects() takes the list it gets from the grammar and passes each
> element to get_object_address().  But a function_with_argtypes_list is a
> list of FuncWithArgs, so we have to make those into single-member lists
> first.  The reason this works for types is that type_name_list looks
> like this:
>
> type_name_list:
> Typename   { $$ = list_make1(list_make1($1)); }
>   | type_name_list ',' Typename{ $$ = lappend($1, list_make1($3)); }
>
> I suppose we could make function_with_argtypes look like that as well
> (and later change it back if we redesign it as discussed above).  I
> think if we did it that way, it would get rid of the warts in this patch
> set.


Moved to next CF with "needs review" status.


Regards,
Hari Babu
Fujitsu Australia

Re: [HACKERS] Patch to implement pg_current_logfile() function

[Haribabu Kommi]

On Fri, Dec 2, 2016 at 12:08 PM, Karl O. Pinc  wrote:

> On Sun, 27 Nov 2016 21:54:46 +0100
> Gilles Darold  wrote:
>
> > I've attached the v15 of the patch
>
> > I've not applied patch patch_pg_current_logfile-v14.diff.backoff to
> > prevent constant call of logfile_writename() on a busy system (errno =
> > ENFILE | EMFILE).
>
> I don't think it should be applied and included in the basic
> functionality patch in any case. I think it needs to be submitted as a
> separate patch along with the basic functionality patch.  Backing off
> the retry of the current_logfiles write could be overly fancy and
> simply not needed.
>
> > I think this can be done quite simply by testing if
> > log rotate is still enabled. This is possible because function
> > logfile_rotate() is already testing if  errno = ENFILE | EMFILE and in
> > this case rotation_disabled is set to true. So the following test
> > should do the work:
> >
> >if (log_metainfo_stale && !rotation_disabled)
> >logfile_writename();
> >
> > This is included in v15 patch.
>
> I don't see this helping much, if at all.
>
> First, it's not clear just when rotation_disabled can be false
> when log_metainfo_stale is true.  The typical execution
> path is for logfile_writename() to be called after rotate_logfile()
> has already set rotataion_disabled to true.   logfile_writename()
> is the only place setting log_metainfo_stale to true and
> rotate_logfile() the only place settig rotation_disabled to false.
>
> While it's possible
> that log_metainfo_stale might be set to true when logfile_writename()
> is called from within open_csvlogfile(), this does not help with the
> stderr case.  IMO better to just test for log_metaifo_stale at the
> code snippet above.
>
> Second, the whole point of retrying the logfile_writename() call is
> to be sure that the current_logfiles file is updated before the logs
> rotate.  Waiting until logfile rotation is enabled defeats the purpose.


Moved to next CF with "needs review" status.

Regards,
Hari Babu
Fujitsu Australia

Re: [HACKERS] s/xlog/wal/ in tools and function names?

[Vladimir Rusinov]

On Fri, Dec 2, 2016 at 2:02 AM Michael Paquier 
wrote:

> On Fri, Dec 2, 2016 at 3:21 AM, Robert Haas  wrote:
> > On Thu, Dec 1, 2016 at 10:29 AM, Vladimir Rusinov 
> wrote:
> >> I've found myself wondering "where is my xlog" after running
> >> pg_switch_xlog() in 10.0.
> >>
> >> Renaming pg_xlog to pg_wal created inconsistency between tools, function
> >> names and directory name on disk.
> >>
> >> Should we also:
> >>
> >> - rename pg_switch_xlog and friends to pg_switch_wal?
> >> - rename pg_recievexlog to pg_revievewal (and others in bin/)?
> >> - rename pg_xlogdump to pg_waldump?
> >
> > I think yes to all.
>
> I was hesitant to propose that, but if there is a will do move
> everything...


I'd like to see this happen, and sounds like so far there is a consensus
that it's a good idea.
Since I'm the one who proposed it, I can send out patches for some or maybe
even eventually all of them.

I guess it would make sense to do all of it in 10.0.
I'm new here, so not very sure about process. How many commit fests could I
expect before 10.0 is out (I'm a bit busy at the moment)?


> Documentation would point to different pages if the
> utilities are renamed, so that's not helpful when comparing features
> across major releases... We may want to keep those files with their
> historical names.
>

I'd rather have html redirects set up (assuming they are technically
possible).
In 10.0+ we'll have pg_receivexlog -> pg_receivewal, and we'll need to have
patches adding redirects pg_receivewal -> pg_receivexlog to older branches
so that people can jump between versions easily on the website.


>
> >> - if we do rename, should we keep aliases for functions and symlinks for
> >> tools?
> >
> > I think no.
>
> Better to do breakages in a single release rather than spreading them
> across releases. While at it and because we are on a crazy trend, one
> thing we could as well consider is removing pg_xlog_location_diff().
> It has lost sense since pg_lsn has been introduced.
>
> >> - anything else?
> >
> > There are some SQL-callable functions that should probably be renamed
> > to match, too.
>
> =# select proname from pg_proc where proname ~ 'xlog';
>  proname
> -
>  pg_current_xlog_location
>  pg_current_xlog_insert_location
>  pg_current_xlog_flush_location
>  pg_xlogfile_name_offset
>  pg_xlogfile_name
>  pg_xlog_location_diff
>  pg_last_xlog_receive_location
>  pg_last_xlog_replay_location
>  pg_is_xlog_replay_paused
>  pg_switch_xlog
>  pg_xlog_replay_pause
>  pg_xlog_replay_resume
> (12 rows)
> --
> Michael
>


smime.p7s
Description: S/MIME Cryptographic Signature

Re: [HACKERS] pgbench more operators & functions

[Fabien COELHO]

Hello,

Sorry for the changing the status of the patch against to the current 
status. While going through the recent mails, I thought that there is 
some disagreement from committer.


If so, I'm willing to explain again why these operators are useful for 
writing some benchmarks, for instance, this paragraph taken randomly from 
the TPC-B specification, on page 16:


"""
  The Account_ID is generated as follows:
  • A random number X is generated within [0,1]
  • If X<0.85 or branches = 1, a random Account_ID is selected over all 
 accounts.
  • If X>=0.85 and branches > 1, a random Account_ID is selected over all 
non- accounts.
"""

This extracy suggests clearly that having some comparison operators and 
some ability to act upon the comparison result is required to implement 
this particular benchmark, which is beyond pgbench current capabilities.



Moved to next CF with "ready for committer" status.


Ok. We'll see next time what becomes of it...

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

Re: [HACKERS] UNDO and in-place update

[Alexander Korotkov]

On Thu, Dec 1, 2016 at 6:25 PM, Robert Haas  wrote:

> There's a couple of possible designs here, but there is the
> possibility for extra hops in some cases.  But there are things we can
> do to mitigate that.
>
> 1. If each tuple has a definitely-all-visible bit, you can check that
> first; if it's set, the tuple is definitely all-visible you don't need
> to do anything further.
>
> 2. If we still maintain a visibility map, you can check that next.  If
> the bit is set for the target page, then the tuple is definitely
> all-visible and you don't need to do anything further.
>
> 3. Otherwise, you need to look at the heap page.  But if the heap
> page's UNDO pointer is invalid, then the whole page is all-visible, so
> you're done.  (You can also set the visibility map bit and/or the
> index tuple's definitely-all-visible bit to avoid having to recheck
> the heap page in the future.)
>
> 4. Each tuple will have a bit indicating whether it's been recently
> modified; that is, whether the transaction whose UNDO log is
> referenced by the page's UNDO pointer did anything to that tuple; or
> if the page points to a TPD entry, whether any of the transactions in
> the TPD modified that tuple.  If that bit is not set for that
> particular tuple, it's all-visible and you're done.
>
> 5. Otherwise you have to look at the TPD entry (if any) and UNDO logs.
>
> If we put in the visibility information in the index as you are
> proposing, then we can always resolve the visibility of the index
> tuple at step 1; we just test xmin and xmax against the snapshot.  For
> index-only scans, that's great, because we never need to consult the
> heap at all.  For regular index scans, it's not nearly as great,
> because we're still going to need to consult the TPD and UNDO logs to
> determine which version of the tuple is visible to that snapshot.  You
> can avoid looking at the heap in the case where no version of the
> tuple is visible to the scan, but that's probably mostly going to
> happen only in cases where the transaction is still in flight so in
> most cases the heap will be in shared_buffers anyway and you won't
> save very much.
>

Idea of storing just one visibility bit in index tuple is a subject of
serious doubts for me.

1. When definitely-all-visible isn't set then we have to recheck during
scanning heap, right?
But our current recheck (i.e. rechecking scan quals) is not enough.
Imagine you have a range
index scan (val >= 100 AND val < 200), and val field was updated from val =
50 and val = 60
in some row.  Then you might have this row duplicated in the output.
Removing index scan and
sticking to only bitmap index scan doesn't seem to be fair.  Thus, we need
to introduce another
kind of recheck that heapTuple.indexKey = indexTuple.indexKey.

2. Another question is how it could work while index key being intensively
updated.  There is a risk
that we would have to traverse same UNDO-chains multiple times.  In worst
case, we would have
to spend quadratic time of number of row versions since our snapshot
taken.  We might try to mitigate this
by caching TID => heap tuple map for our snapshot.  But I don't like this
approach.
If we have large enough index scan, then we could either run out of cache
or consume too much memory
for that cache.

So, I think storing visibility information in index tuple is great for
regular index scans too.  At least,
it allow us to evade #2 problem.  That is great by itself.

--
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Re: [HACKERS] Parallel execution and prepared statements

[Tobias Bussmann]

> On Thu, Dec 1, 2016 at 9:40 PM, Robert Haas  wrote:
>> 
>> OK, then my vote is to do it that way for now.

Thanks for your opinion. That's fine with me.

> Am 02.12.2016 um 07:22 schrieb Amit Kapila :
> Done that way in attached patch.

Did a quick review: The patch applies cleanly against current head. make 
installcheck with force_parallel_mode = regress passes all tests. My manual 
tests show that parallel query is working for prepared statements in SQL with 
PREPARE and EXECUTE. CREATE TABLE AS EXECUTE is working, EXPLAIN on that shows 
a parallel plan, EXPLAIN ANALZE indicates 0 launched workers for that. Looks 
fine so far!

You should however include a sentence in the documentation on that parallel 
plan w/o workers corner-case behaviour. Feel free to take that from my patch or 
phase a better wording.

And again my question regarding back patching to 9.6: 
- 9.6 is currently broken as Laurenz showed in [1]
- 9.6 does not have documented that SQL PREPARE prepared statements cannot not 
use parallel query

The former could be fixed by back patching the full patch which would void the 
latter. Or it could be fixed by disabling generation of parallel plans in 
extended query protocol prepare. Alternatively only the change in execMain.c 
could be back patched. In these cases we would need to have the a separate 
wording for the 9.6 docs.

Best regards,
Tobias

[1] a737b7a37273e048b164557adef4a58b53999...@ntex2010i.host.magwien.gv.at

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