Re: [HACKERS] Covering + unique indexes

[Andreas Joseph Krogh]

På lørdag 09. april 2016 kl. 06:34:39, skrev Tom Lane mailto:t...@sss.pgh.pa.us>>:
Andreas Joseph Krogh  writes:
 > Any reason $subject didn't make it (commited but reverted)?

 See the thread on -committers.
 
Ah, thanks.
 
-- Andreas Joseph Krogh

Re: [HACKERS] Covering + unique indexes

[Tom Lane]

Andreas Joseph Krogh  writes:
> Any reason $subject didn't make it (commited but reverted)?

See the thread on -committers.

regards, tom lane


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

[HACKERS] Covering + unique indexes

[Andreas Joseph Krogh]

Any reason $subject didn't make it (commited but reverted)?
 
This is a great feature and lots of work seems to have been put into this 
patch along with quite some reviewing. It would be nice to know why -hackers 
think it's not ready for 9.6.
 
Thanks.
 
-- Andreas Joseph Krogh

Re: [HACKERS] Support for N synchronous standby servers - take 2

[Tom Lane]

Jeff Janes  writes:
> When I compile now without cassert, I get the compiler warning:

> syncrep.c: In function 'SyncRepUpdateConfig':
> syncrep.c:878:6: warning: variable 'parse_rc' set but not used
> [-Wunused-but-set-variable]

If there's a good reason for that to be an Assert, I don't see it.
There are no callers of SyncRepUpdateConfig that look like they
need to, or should expect not to have to, tolerate errors.
I think the way to fix this is to turn the Assert into a plain
old test-and-ereport-ERROR.

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] the buildfarm has measles

[Andres Freund]

On April 8, 2016 8:05:31 PM PDT, Tom Lane  wrote:
>Alvaro Herrera  writes:
>> Robert Haas wrote:
>>> woodlouse and thrips failed like this (and mastodon in a similar but
>>> not identical way):
>>> 
>>> "C:\buildfarm\buildenv\HEAD\pgsql.build\pgsql.sln" (Standardziel)
>(1) ->
>>> "C:\buildfarm\buildenv\HEAD\pgsql.build\ascii_and_mic.vcxproj"
>>> (Standardziel) (2) ->
>>> "C:\buildfarm\buildenv\HEAD\pgsql.build\postgres.vcxproj"
>(Standardziel) (3) ->
>>> (ClCompile Ziel) ->
>>> src/backend/access/brin/brin_pageops.c(824): error C2026: string too
>>> big, trailing characters truncated
>>> [C:\buildfarm\buildenv\HEAD\pgsql.build\postgres.vcxproj]
>
>> This complaint refers to this line:
>
>>  Assert(BRIN_IS_REGULAR_PAGE(BufferGetPage(oldbuf, NULL, NULL,
>> BGP_NO_SNAPSHOT_TEST)));
>
>> which the compiler complains that gets too long.  This is related:
>> https://www.postgresql.org/message-id/4407.1435763473%40sss.pgh.pa.us
>
>Yeah, pademelon thinks it's too long as well.
>
>> I suggest that we fix this by making BufferGetPage an inline function
>> rather than a macro.
>
>+1

I'll get back to that patch early needy week.

Andres
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.


-- 
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] the buildfarm has measles

[Tom Lane]

Robert Haas  writes:
> prairiedog is not happy about the new plpython error stuff.

Yeah, see my complaint here:
http://www.postgresql.org/message-id/12559.1460141...@sss.pgh.pa.us

I think it's just a matter of avoiding invented-in-this-decade
Pythonisms.  The feature doesn't seem to be broken in itself on
that old Python version, it's just the test that's bad.  I think
we can await a rewritten test case from Pavel.

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] the buildfarm has measles

[Tom Lane]

Alvaro Herrera  writes:
> Robert Haas wrote:
>> woodlouse and thrips failed like this (and mastodon in a similar but
>> not identical way):
>> 
>> "C:\buildfarm\buildenv\HEAD\pgsql.build\pgsql.sln" (Standardziel) (1) ->
>> "C:\buildfarm\buildenv\HEAD\pgsql.build\ascii_and_mic.vcxproj"
>> (Standardziel) (2) ->
>> "C:\buildfarm\buildenv\HEAD\pgsql.build\postgres.vcxproj" (Standardziel) (3) 
>> ->
>> (ClCompile Ziel) ->
>> src/backend/access/brin/brin_pageops.c(824): error C2026: string too
>> big, trailing characters truncated
>> [C:\buildfarm\buildenv\HEAD\pgsql.build\postgres.vcxproj]

> This complaint refers to this line:

>   Assert(BRIN_IS_REGULAR_PAGE(BufferGetPage(oldbuf, NULL, NULL,
>  BGP_NO_SNAPSHOT_TEST)));

> which the compiler complains that gets too long.  This is related:
> https://www.postgresql.org/message-id/4407.1435763473%40sss.pgh.pa.us

Yeah, pademelon thinks it's too long as well.

> I suggest that we fix this by making BufferGetPage an inline function
> rather than a macro.

+1

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] the buildfarm has measles

[Andrew Dunstan]

On 04/08/2016 10:38 PM, Andrew Dunstan wrote:



On 04/08/2016 07:57 PM, Robert Haas wrote:

bowerbird and lorikeet are complaining about the git tree being dirty.



bowerbird is fixed and rebuilding.






lorikeet should be fixed, too.

But bowerbird failed compiling an Assert from commit 8b65cf4c (Modify 
BufferGetPage() to prepare for "snapshot too old" feature).


The error messsage is:


src/backend/access/brin/brin_pageops.c(824): error C2026: string too 
big, trailing characters truncated


Not sure what that's about.

cheers

andrew




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

Re: [HACKERS] the buildfarm has measles

[Alvaro Herrera]

Alvaro Herrera wrote:
> Robert Haas wrote:
> 
> > crake failed 'make check':
> > 
> > *** /home/bf/bfr/root/HEAD/pgsql.build/src/test/regress/expected/misc.out
> > Fri Apr  8 19:38:58 2016
> > --- /home/bf/bfr/root/HEAD/pgsql.build/src/test/regress/results/misc.out
> > Fri Apr  8 19:39:12 2016
> > ***
> > *** 607,612 
> > --- 607,613 
> >circle_tbl
> >city
> >copy_tbl
> > +  ctv_data
> >d
> >d_star
> >date_tbl
> 
> Ahh, my bug -- this is because test psql, in the same parallel group as
> misc, creates this table.  I hesitate to move the whole test to another
> group; perhaps it's better to split the crosstabview out to its own
> file, as Daniel had submitted it.

Pushed.

-- 
Álvaro Herrerahttp://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] the buildfarm has measles

[Andrew Dunstan]

On 04/08/2016 07:57 PM, Robert Haas wrote:

bowerbird and lorikeet are complaining about the git tree being dirty.



bowerbird is fixed and rebuilding.

cheers

andrew




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

Re: [HACKERS] [COMMITTERS] pgsql: Add the "snapshot too old" feature

[Alvaro Herrera]

Kevin Grittner wrote:
> Add the "snapshot too old" feature
> 
> This feature is controlled by a new old_snapshot_threshold GUC.  A
> value of -1 disables the feature, and that is the default.  The
> value of 0 is just intended for testing.  Above that it is the
> number of minutes a snapshot can reach before pruning and vacuum
> are allowed to remove dead tuples which the snapshot would
> otherwise protect.  The xmin associated with a transaction ID does
> still protect dead tuples.  A connection which is using an "old"
> snapshot does not get an error unless it accesses a page modified
> recently enough that it might not be able to produce accurate
> results.

I think this formulation of TestForOldSnapshot as returning the Page it
checks is a bit strange; you seem to have done it that way only to be
able to write BufferGetPage in a reasonable manner.  I vote for changing
both those macros into inline functions instead, pursuant to
https://www.postgresql.org/message-id/20160409020835.GA727750%40alvherre.pgsql
and have TestForOldSnapshot return void.

-- 
Álvaro Herrerahttp://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] Support for N synchronous standby servers - take 2

[Jeff Janes]

On Wed, Apr 6, 2016 at 1:23 AM, Fujii Masao  wrote:

> Okay, I pushed the patch!
> Many thanks to all involved in the development of this feature!

Thanks, a nice feature.

When I compile now without cassert, I get the compiler warning:

syncrep.c: In function 'SyncRepUpdateConfig':
syncrep.c:878:6: warning: variable 'parse_rc' set but not used
[-Wunused-but-set-variable]

Cheers,

Jeff


-- 
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] Speedup twophase transactions

[Noah Misch]

On Fri, Apr 08, 2016 at 02:57:00PM -0400, Jesper Pedersen wrote:
> On 04/08/2016 02:37 PM, Robert Haas wrote:
> >On Fri, Apr 8, 2016 at 8:49 AM, Jesper Pedersen  
> >wrote:
> >>Should we create an entry for the open item list [0] for this, due to the
> >>replication lag [1] ?
> >>
> >>CommitFest entry [2]
> >>Original commit [3]
> >>
> >>Cc'ed RMT.
> >
> >If there is something you think needs to be fixed that is a new issue
> >in 9.6, then yes you should.  I don't quite understand what thing is
> >from reading this, so please make sure to describe it clearly.
> >
> 
> Michael, you seem to have the necessary permission for this. Could you add
> an entry ?

Everyone may edit the list; follow
https://wiki.postgresql.org/wiki/WikiEditing to setup access.


-- 
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] the buildfarm has measles

[Alvaro Herrera]

Robert Haas wrote:

> woodlouse and thrips failed like this (and mastodon in a similar but
> not identical way):
> 
> "C:\buildfarm\buildenv\HEAD\pgsql.build\pgsql.sln" (Standardziel) (1) ->
> "C:\buildfarm\buildenv\HEAD\pgsql.build\ascii_and_mic.vcxproj"
> (Standardziel) (2) ->
> "C:\buildfarm\buildenv\HEAD\pgsql.build\postgres.vcxproj" (Standardziel) (3) 
> ->
> (ClCompile Ziel) ->
>   src/backend/access/brin/brin_pageops.c(824): error C2026: string too
> big, trailing characters truncated
> [C:\buildfarm\buildenv\HEAD\pgsql.build\postgres.vcxproj]

This complaint refers to this line:

Assert(BRIN_IS_REGULAR_PAGE(BufferGetPage(oldbuf, NULL, NULL,
   BGP_NO_SNAPSHOT_TEST)));

which the compiler complains that gets too long.  This is related:
https://www.postgresql.org/message-id/4407.1435763473%40sss.pgh.pa.us
(There's a patch there which was never pushed AFAICS).

I suggest that we fix this by making BufferGetPage an inline function
rather than a macro.

> crake failed 'make check':
> 
> *** /home/bf/bfr/root/HEAD/pgsql.build/src/test/regress/expected/misc.out
> Fri Apr  8 19:38:58 2016
> --- /home/bf/bfr/root/HEAD/pgsql.build/src/test/regress/results/misc.out
> Fri Apr  8 19:39:12 2016
> ***
> *** 607,612 
> --- 607,613 
>circle_tbl
>city
>copy_tbl
> +  ctv_data
>d
>d_star
>date_tbl

Ahh, my bug -- this is because test psql, in the same parallel group as
misc, creates this table.  I hesitate to move the whole test to another
group; perhaps it's better to split the crosstabview out to its own
file, as Daniel had submitted it.

-- 
Álvaro Herrerahttp://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] closing CommitFest 2016-03, feature freeze now in effect

[Joshua D. Drake]

On 04/08/2016 05:05 PM, Robert Haas wrote:

CommitFest 2016-03 is now closed.  I have moved "Twophase transactions
on slave", "Partial sort", and "amcheck (B-Tree integrity checking
tool)" to the next CommitFest in accordance with the policy previous
set by the release management team.   I have left "Replace buffer
manager spinlock with atomic operations" active in the current
CommitFest because it was granted an extension.  The RMT has received
Tom's request for an extension on the "Unique Joins" patch but has not
yet reached a decision.

Feature freeze is now in effect.  Please, no more feature commits.
Let's turn our attention to the task of working through the open items
list.

I think this is going to be a great release.  Hopefully, we (by which
I mean, in no small part, I) have not committed too many bugs along
with all of the great features.

Thanks,



Can I just say, "Woot!, well done all!"

Sincerely,

JD

--
Command Prompt, Inc.  http://the.postgres.company/
+1-503-667-4564
PostgreSQL Centered full stack support, consulting and development.
Everyone appreciates your honesty, until you are honest with them.


--
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] VS 2015 support in src/tools/msvc

[Petr Jelinek]

On 09/04/16 00:41, Michael Paquier wrote:

On Sat, Apr 9, 2016 at 1:46 AM, Christian Ullrich  wrote:

* Andrew Dunstan wrote:


On 04/08/2016 11:02 AM, Christian Ullrich wrote:




   src/port/chklocale.c(233): warning C4133: 'function': incompatible
   types - from 'const char *' to 'LPCWSTR' [...\postgres.vcxproj]




Do you have a fix for the LPCWSTR parameter issue?



As long as the locale short name cannot contain characters outside of ASCII,
and I don't see how it could, just the typical measure-allocate-convert
dance, add error handling to taste:

int res = MultiByteToWideChar(CP_ACP, 0, ctype, -1, NULL, 0);
WCHAR *wctype = malloc(res * sizeof(WCHAR));
memset(wctype, 0, res * sizeof(WCHAR));
res = MultiByteToWideChar(CP_ACP, 0, ctype, -1, wctype, wctypelen);

If it is somehow guaranteed that ctype is only the most basic short name
("xx-YY") with no code pages or anything, it becomes much simpler, of
course, and I would just use a loop.

If the locale name can contain characters above 0x7f, we'd have to know the
code page of the string we use to get the code page.


Could somebody give a try instead of me? I could take a look on it,
but just in 12 hours or so, aka after the deadline if that matters for
this patch.



I won't be able to get back to it (well mainly to windows environment) 
till Thursday due to travel, sorry.


--
  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

[HACKERS] closing CommitFest 2016-03, feature freeze now in effect

[Robert Haas]

CommitFest 2016-03 is now closed.  I have moved "Twophase transactions
on slave", "Partial sort", and "amcheck (B-Tree integrity checking
tool)" to the next CommitFest in accordance with the policy previous
set by the release management team.   I have left "Replace buffer
manager spinlock with atomic operations" active in the current
CommitFest because it was granted an extension.  The RMT has received
Tom's request for an extension on the "Unique Joins" patch but has not
yet reached a decision.

Feature freeze is now in effect.  Please, no more feature commits.
Let's turn our attention to the task of working through the open items
list.

I think this is going to be a great release.  Hopefully, we (by which
I mean, in no small part, I) have not committed too many bugs along
with all of the great features.

Thanks,

-- 
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] the buildfarm has measles

[Robert Haas]

bowerbird and lorikeet are complaining about the git tree being dirty.

prairiedog is not happy about the new plpython error stuff.

woodlouse and thrips failed like this (and mastodon in a similar but
not identical way):

"C:\buildfarm\buildenv\HEAD\pgsql.build\pgsql.sln" (Standardziel) (1) ->
"C:\buildfarm\buildenv\HEAD\pgsql.build\ascii_and_mic.vcxproj"
(Standardziel) (2) ->
"C:\buildfarm\buildenv\HEAD\pgsql.build\postgres.vcxproj" (Standardziel) (3) ->
(ClCompile Ziel) ->
  src/backend/access/brin/brin_pageops.c(824): error C2026: string too
big, trailing characters truncated
[C:\buildfarm\buildenv\HEAD\pgsql.build\postgres.vcxproj]

crake failed 'make check':

*** /home/bf/bfr/root/HEAD/pgsql.build/src/test/regress/expected/misc.out
Fri Apr  8 19:38:58 2016
--- /home/bf/bfr/root/HEAD/pgsql.build/src/test/regress/results/misc.out
Fri Apr  8 19:39:12 2016
***
*** 607,612 
--- 607,613 
   circle_tbl
   city
   copy_tbl
+  ctv_data
   d
   d_star
   date_tbl
***
*** 710,716 
   tvvmv
   varchar_tbl
   xacttest
! (132 rows)

  SELECT name(equipment(hobby_construct(text 'skywalking', text 'mer')));
   name
--- 711,717 
   tvvmv
   varchar_tbl
   xacttest
! (133 rows)

  SELECT name(equipment(hobby_construct(text 'skywalking', text 'mer')));
   name

-- 
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] Weighted Stats

[David Fetter]

On Fri, Mar 18, 2016 at 06:12:12PM -0700, Jeff Janes wrote:
> On Tue, Mar 15, 2016 at 8:36 AM, David Fetter  wrote:
> >
> > Please find attached a patch that uses the float8 version to cover the
> > numeric types.
> 
> Is there a well-defined meaning for having a negative weight?  If no,
> should it be disallowed?

Done.

> Shouldn't these then give the same result:
> 
> select stddev_samp(val) from foo;
> stddev_samp
> ---
>  2887.054977297105
> 
> select weighted_stddev_samp(val,count) from foo2;
>  weighted_stddev_samp
> --
>  2887.19919651336
> 
> The 5th digit seems too early to be seeing round-off error.

Fixed down-thread.

Cheers,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate


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

Re: [HACKERS] [patch] Proposal for \crosstabview in psql

[Alvaro Herrera]

Daniel Verite wrote:

> ISTM that this could be avoided by erroring out for lack of an
> explicit 3rd column as argument. IOW, we wouldn't assume
> that "no column specified" means "show all columns". 
> 
> About simply ripping out the possibility of having multiple
> columns into cells, it's more radical but if that part turns out to
> be more confusing than useful, I don't have a problem
> with removing it.

Okay, I've ripped that out since I wasn't comfortable with the general
idea.  Once you have two data values for the same cell, the new code
raises an error, indicating the corresponding vertical and horizontal
header values; that way it's easy to spot where the problem is.

I also removed the FETCH_COUNT bits; it didn't make a lot of sense to
me.  Like \gexec, the query is executed to completion when in
\crosstabview regardless of FETCH_COUNT.

> The other case of stringing multiple contents into the same cell
> is when different tuples carry (row,column) duplicates.
> I'm not inclined to disallow that case, I think it would go too far
> in guessing what the user expects.
> My expectation for a viewer is that it displays the results as far as
> possible, whatever they are. 

The reason I made this case throw an error is that we can tweak the
behavior later.  I think separating them with newlines is too cute and
will be unusable when you have values that have embedded newlines; you
can imitate that behavior with string_agg(val, E'\n') as I've done in
the regression tests.  One option for improving it would be to have it
add another record, but that requires shifting the values of all cells
by the number of columns (you can see that if you change the border
options, or in HTML output etc).  We can do that later.

> Also, showing such contents in vertically-growing cells as it
> does now allows the user to spot these easily in the grid when
> they happen to be outliers. I'm seeing it as useful in that case.

It's useful, no doubt.

I pushed it.

-- 
Álvaro Herrerahttp://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] multivariate statistics v14

[Tatsuo Ishii]

From: Tomas Vondra 
Subject: Re: [HACKERS] multivariate statistics v14
Date: Fri, 8 Apr 2016 20:55:24 +0200
Message-ID: <5d1d62a6-6228-188c-e079-c1be59942...@2ndquadrant.com>

> On 04/08/2016 05:55 PM, Robert Haas wrote:
>> On Tue, Mar 29, 2016 at 11:18 AM, David Steele 
>> wrote:
>>> On 3/28/16 4:42 AM, Tomas Vondra wrote:
 Yes, those are valid omissions. I plan to address them, and I'd also
 considering adding a section to 65.1 (How the Planner Uses
 Statistics),
 explaining more thoroughly how the planner uses multivariate stats.
>>>
>>> It looks you need post a new patch so I have marked this "waiting on
>>> author".
>>
>> Since no new version of this patch has been posted in the last 10
>> days, it seems clear that there will not be time for this to
>> reasonably become ready for committer and then get committed in the
>> few hours remaining before the deadline. That is a bummer, since I
>> was hoping we would have this feature in this release, but hopefully
>> we will get it into 9.7. I am marking it Returned with Feedback.
>>
> 
> Well, me to. But my feeling is the patch received entirely
> insufficient amount of thorough code review, considering how important
> part of the code it touches. I agree docs are an important part of a
> patch, but polishing user-level docs would hardly move the patch
> closer to being committable (especially when there's ~50kB of
> READMEs).

My feedback regarding docs were:
> - There's no docs for pg_mv_statistic (should be added to "49. System
>   Catalogs")
>
> - The word "multivariate statistics" or something like that should
>   appear in the index.
> 
> - There are some explanation how to deal with multivariate statistics
>   in "14.1 Using Explain" and "14.2 Statistics used by the Planner"
>   section.

The second and the third point maybe are something like "polishing
user-level" docs, but I don't think the first one is for "user-level".
Also I think without the first one the patch will be never
committable. If someone add a new system catalog, the doc should be
added to "System Catalogs" section, that's our standard, at least in
my understanding.

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp


-- 
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] Weighted Stats

[David Fetter]

On Fri, Apr 08, 2016 at 01:47:56PM -0700, David Fetter wrote:
> Sorry about the delay.  This patch disallows negative weights,
> although it still has that odd difference Jeff found.

Difference corrected.  It turned out that my reference was mistaken on
the calculation.

Cheers,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index cb7fe38..ba31d02 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -12750,6 +12750,29 @@ NULL baz(3 rows)
  
   

+weighted_average
+   
+   
+weighted_avg
+   
+   weighted_avg(value 
expression, weight 
expression)
+  
+  
+   smallint, int,
+   bigint, real, double
+   precision, numeric, or interval
+  
+  
+   numeric for any integer-type argument,
+   double precision for a floating-point argument,
+   otherwise the same as the argument data type
+  
+  the average (arithmetic mean) of all input values, weighted by 
the input weights
+ 
+
+ 
+  
+   
 bit_and

bit_and(expression)
@@ -13430,6 +13453,29 @@ SELECT xmlagg(x) FROM (SELECT x FROM test ORDER BY y 
DESC) AS tab;
  
   

+weighted standard deviation
+population
+   
+   
+weighted_stddev_pop
+   
+   weighted_stddev_pop(value 
expression, weight 
expression)
+  
+  
+   smallint, int,
+   bigint, real, double
+   precision, or numeric
+  
+  
+   double precision for floating-point arguments,
+   otherwise numeric
+  
+  weighted population standard deviation of the input values
+ 
+
+ 
+  
+   
 standard deviation
 sample

@@ -13454,6 +13500,29 @@ SELECT xmlagg(x) FROM (SELECT x FROM test ORDER BY y 
DESC) AS tab;
  
   

+weighted standard deviation
+sample
+   
+   
+weighted_stddev_samp
+   
+   weighted_stddev_samp(value 
expression, weight 
expression)
+  
+  
+   smallint, int,
+   bigint, real, double
+   precision, or numeric
+  
+  
+   double precision for floating-point arguments,
+   otherwise numeric
+  
+  weighted sample standard deviation of the input values
+ 
+
+ 
+  
+   
 variance

variance(expression)
diff --git a/src/backend/utils/adt/float.c b/src/backend/utils/adt/float.c
index c7c0b58..fe4a5e4 100644
--- a/src/backend/utils/adt/float.c
+++ b/src/backend/utils/adt/float.c
@@ -2405,6 +2405,7 @@ setseed(PG_FUNCTION_ARGS)
  * float8_accum- accumulate for AVG(), variance 
aggregates, etc.
  * float4_accum- same, but input data is float4
  * float8_avg  - produce final result for 
float AVG()
+ * float8_weighted_avg - produce final result for float 
WEIGHTED_AVG()
  * float8_var_samp - produce final result for float 
VAR_SAMP()
  * float8_var_pop  - produce final result for float 
VAR_POP()
  * float8_stddev_samp  - produce final result for float 
STDDEV_SAMP()
@@ -3205,6 +3206,163 @@ float8_regr_intercept(PG_FUNCTION_ARGS)
PG_RETURN_FLOAT8(numeratorXXY / numeratorX);
 }
 
+/*
+ * ===
+ * WEIGHTED AGGREGATES
+ * ===
+ *
+ * The transition datatype for these aggregates is a 5-element array
+ * of float8, holding the values N, sum(W), sum(W*X), and sum(W*X*X)
+ * in that order.
+ *
+ * First, an accumulator function for those we can't pirate from the
+ * other accumulators.  This accumulator function takes out some of
+ * the rounding error inherent in the general one.
+ * https://en.wikipedia.org/wiki/Standard_deviation#Rapid_calculation_methods
+ *
+ * It consists of a five-element array which includes:
+ *
+ * N, the number of non-zero-weighted values seen thus far,
+ * W, the running sum of weights,
+ * WX, the running dot product of weights and values,
+ * A, an intermediate value used in the calculation, and
+ * Q, another intermediate value.
+ *
+ */
+
+Datum
+float8_weighted_accum(PG_FUNCTION_ARGS)
+{
+   ArrayType  *transarray = PG_GETARG_ARRAYTYPE_P(0);
+   float8  newvalX = PG_GETARG_FLOAT8(1);
+   float8  newvalW = PG_GETARG_FLOAT8(2);
+   float8 *transvalues;
+   float8  N,  /* common */
+   W,  /* common */
+   WX, /* Used in avg */
+   A,  /* Used in stddev_* */
+   Q;  /* Used in stddev_* */
+
+   tr

Re: [HACKERS] VS 2015 support in src/tools/msvc

[Michael Paquier]

On Sat, Apr 9, 2016 at 1:46 AM, Christian Ullrich  wrote:
> * Andrew Dunstan wrote:
>
>> On 04/08/2016 11:02 AM, Christian Ullrich wrote:
>
>
>>>   src/port/chklocale.c(233): warning C4133: 'function': incompatible
>>>   types - from 'const char *' to 'LPCWSTR' [...\postgres.vcxproj]
>
>
>> Do you have a fix for the LPCWSTR parameter issue?
>
>
> As long as the locale short name cannot contain characters outside of ASCII,
> and I don't see how it could, just the typical measure-allocate-convert
> dance, add error handling to taste:
>
> int res = MultiByteToWideChar(CP_ACP, 0, ctype, -1, NULL, 0);
> WCHAR *wctype = malloc(res * sizeof(WCHAR));
> memset(wctype, 0, res * sizeof(WCHAR));
> res = MultiByteToWideChar(CP_ACP, 0, ctype, -1, wctype, wctypelen);
>
> If it is somehow guaranteed that ctype is only the most basic short name
> ("xx-YY") with no code pages or anything, it becomes much simpler, of
> course, and I would just use a loop.
>
> If the locale name can contain characters above 0x7f, we'd have to know the
> code page of the string we use to get the code page.

Could somebody give a try instead of me? I could take a look on it,
but just in 12 hours or so, aka after the deadline if that matters for
this patch.
-- 
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] [COMMITTERS] pgsql: Add the "snapshot too old" feature

[Kevin Grittner]

On Fri, Apr 8, 2016 at 2:45 PM, Kevin Grittner  wrote:
> Add the "snapshot too old" feature

> src/test/modules/Makefile  |   1 +
> src/test/modules/snapshot_too_old/Makefile |  47 +++
> .../snapshot_too_old/expected/sto_using_cursor.out |  73 
> .../snapshot_too_old/expected/sto_using_select.out |  55 +++
> .../snapshot_too_old/specs/sto_using_cursor.spec   |  37 ++
> .../snapshot_too_old/specs/sto_using_select.spec   |  36 ++
> src/test/modules/snapshot_too_old/sto.conf |   3 +

I see that there are failures on buildfarm Windows machines.
Makefiles are not my strong suit, and I don't have access to a
Windows machine for builds.  I'll start digging, but if someone can
help me with that, it would be much appreciated.

--
Kevin Grittner
EDB: 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] Re: [COMMITTERS] pgsql: Add trigonometric functions that work in degrees.

[Tom Lane]

Peter Eisentraut  writes:
> On 01/22/2016 03:46 PM, Tom Lane wrote:
>> Add trigonometric functions that work in degrees.

> I have a host here that is having regression test failures from this commit:

> --- src/test/regress/expected/float8.out
> +++ src/test/regress/results/float8.out
> @@ -490,9 +490,9 @@
> x   | asind | acosd | atand
>   --+---+---+---
>  -1 |   -90 |   180 |   -45
> - -0.5 |   -30 |   120 |
> + -0.5 |   |   120 |
>   0 | 0 |90 | 0
> -  0.5 |30 |60 |
> +  0.5 |   |60 |
>   1 |90 | 0 |45
>   (5 rows)

BTW ... looking closer at that, it appears to show asind(-0.5) and
asind(0.5) returning NULL.  Which makes no sense at all, because
there is no provision in dasind() for returning a null, regardless
of the input value.

So I'm pretty baffled.  Maybe you could step through this and figure
out where it's going off the rails?

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] Default Roles (was: Additional role attributes)

[Stephen Frost]

Noah, Fujii, all,

* Noah Misch (n...@leadboat.com) wrote:
> At the C level, have a pgstattuple function and a pgstattuple_v1_4 function.
> Let them differ only in that the former has a superuser check.  Binary
> upgrades will use the former, and fresh CREATE EXTENSION shall use the latter.

Attached is a patch which implements this for the pgstattuple
extensions.  The changes are pretty straight-forward, but I'm not going
to commit this under the gun of the feature freeze without at least
another committer reviewing it or getting an extension for a couple days
to play with it further and convince myself it's safe.

Ultimately, I'd like for this to be included in 9.6 as it'd be an
example use-case for others to follow when updating their extensions to
make use of the new pg_dump features, but I certainly don't see it as
being critical to the release.

Fujii, my apologies for not getting this done earlier, I know this is a
capability you are looking forward to having.

Thanks!

Stephen
From 0db7ebf549aeee7f04b8383ac391f349f810ef4b Mon Sep 17 00:00:00 2001
From: Stephen Frost 
Date: Fri, 8 Apr 2016 17:18:27 -0400
Subject: [PATCH] Remove superuser checks in pgstattuple 1.4

Now that we track initial privileges on extension objects and changes to
those permissions, we can drop the superuser() checks from the various
functions which are part of the pgstattuple extension.

Since a pg_upgrade will preserve the version of the extension which
existed prior to the upgrade, we can't simply modify the existing
functions but instead need to create new functions which remove the
checks and update the SQL-level functions to use the new functions
(and to REVOKE EXECUTE rights on those functions from PUBLIC).

Approach suggested by Noah.
---
 contrib/pgstattuple/Makefile  |   2 +-
 contrib/pgstattuple/pgstatapprox.c|  35 ++--
 contrib/pgstattuple/pgstatindex.c | 108 +++--
 contrib/pgstattuple/pgstattuple--1.3--1.4.sql | 111 ++
 contrib/pgstattuple/pgstattuple--1.3.sql  |  95 --
 contrib/pgstattuple/pgstattuple--1.4.sql  | 111 ++
 contrib/pgstattuple/pgstattuple.c |  36 +
 contrib/pgstattuple/pgstattuple.control   |   2 +-
 8 files changed, 392 insertions(+), 108 deletions(-)
 create mode 100644 contrib/pgstattuple/pgstattuple--1.3--1.4.sql
 delete mode 100644 contrib/pgstattuple/pgstattuple--1.3.sql
 create mode 100644 contrib/pgstattuple/pgstattuple--1.4.sql

diff --git a/contrib/pgstattuple/Makefile b/contrib/pgstattuple/Makefile
index 6083dab..01f1feb 100644
--- a/contrib/pgstattuple/Makefile
+++ b/contrib/pgstattuple/Makefile
@@ -4,7 +4,7 @@ MODULE_big	= pgstattuple
 OBJS		= pgstattuple.o pgstatindex.o pgstatapprox.o $(WIN32RES)
 
 EXTENSION = pgstattuple
-DATA = pgstattuple--1.3.sql pgstattuple--1.2--1.3.sql pgstattuple--1.1--1.2.sql pgstattuple--1.0--1.1.sql pgstattuple--unpackaged--1.0.sql
+DATA = pgstattuple--1.4.sql pgstattuple--1.3--1.4.sql pgstattuple--1.2--1.3.sql pgstattuple--1.1--1.2.sql pgstattuple--1.0--1.1.sql pgstattuple--unpackaged--1.0.sql
 PGFILEDESC = "pgstattuple - tuple-level statistics"
 
 REGRESS = pgstattuple
diff --git a/contrib/pgstattuple/pgstatapprox.c b/contrib/pgstattuple/pgstatapprox.c
index b7734fa..5671791 100644
--- a/contrib/pgstattuple/pgstatapprox.c
+++ b/contrib/pgstattuple/pgstatapprox.c
@@ -29,6 +29,9 @@
 #include "commands/vacuum.h"
 
 PG_FUNCTION_INFO_V1(pgstattuple_approx);
+PG_FUNCTION_INFO_V1(pgstattuple_approx_v1_4);
+
+Datum pgstattuple_approx_internal(Oid relid, FunctionCallInfo fcinfo);
 
 typedef struct output_type
 {
@@ -209,6 +212,33 @@ Datum
 pgstattuple_approx(PG_FUNCTION_ARGS)
 {
 	Oid			relid = PG_GETARG_OID(0);
+
+	if (!superuser())
+		ereport(ERROR,
+(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ (errmsg("must be superuser to use pgstattuple functions";
+
+	PG_RETURN_DATUM(pgstattuple_approx_internal(relid, fcinfo));
+}
+
+/*
+ * As of pgstattuple version 1.4, we no longer need to check if the user
+ * is a superuser because we REVOKE EXECUTE on the function from PUBLIC.
+ * Users can then grant access to it based on their policies.
+ *
+ * Otherwise identical to pgstattuple_approx (above).
+ */
+Datum
+pgstattuple_approx_v1_4(PG_FUNCTION_ARGS)
+{
+	Oid			relid = PG_GETARG_OID(0);
+
+	PG_RETURN_DATUM(pgstattuple_approx_internal(relid, fcinfo));
+}
+
+Datum
+pgstattuple_approx_internal(Oid relid, FunctionCallInfo fcinfo)
+{
 	Relation	rel;
 	output_type stat = {0};
 	TupleDesc	tupdesc;
@@ -217,11 +247,6 @@ pgstattuple_approx(PG_FUNCTION_ARGS)
 	HeapTuple	ret;
 	int			i = 0;
 
-	if (!superuser())
-		ereport(ERROR,
-(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
- (errmsg("must be superuser to use pgstattuple functions";
-
 	if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
 		elog(ERROR, "return type must be a row type");
 
diff --git a/contrib/pgstattuple/pgstatinde

Re: [HACKERS] snapshot too old, configured by time

[David Steele]

On 4/8/16 4:30 PM, Peter Geoghegan wrote:
> On Fri, Apr 8, 2016 at 12:55 PM, Kevin Grittner  wrote:
>> Sadly, I forgot to include the reviewer information when writing the
>> commit messages.  :-(
> 
> Oh well. I'm just glad we got the patch over the line. I think that
> there are some types of users that will very significantly benefit
> from this patch.

I'm also very happy to see this go in.  While I used to dread the
"snapshot too old" error back in my Oe architect days it's nice to
have the option, especially when it can be configured by time rather
than by size.

-- 
-David
da...@pgmasters.net


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

Re: [HACKERS] pg_hba_lookup function to get all matching pg_hba.conf entries

[Robert Haas]

On Fri, Apr 8, 2016 at 4:36 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> On Fri, Apr 8, 2016 at 3:24 PM, David Steele  wrote:
>>> Can one of the reviewers decide if this is ready to commit?  I fear it
>>> will be pushed to the next CF otherwise.  I don't think the committers
>>> have time to make that determination today...
>
>> Well, it's not getting committed unless some committer determines that
>> it is ready to commit.
>
> I took a quick look; IMO it is certainly not ready to commit.

OK, marked Returned with Feedback.  Hopefully the patch authors will
find your feedback useful.

-- 
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] Weighted Stats

[David Fetter]

On Sun, Mar 20, 2016 at 03:38:40PM -0700, David Fetter wrote:
> On Sat, Mar 19, 2016 at 05:04:08PM +0100, Tomas Vondra wrote:
> > Hi,
> > 
> > On 03/19/2016 07:34 AM, David Fetter wrote:
> > >On Fri, Mar 18, 2016 at 06:12:12PM -0700, Jeff Janes wrote:
> > >>On Tue, Mar 15, 2016 at 8:36 AM, David Fetter  wrote:
> > >>>
> > >>>Please find attached a patch that uses the float8 version to cover the
> > >>>numeric types.
> > >>
> > >>Is there a well-defined meaning for having a negative weight?  If no,
> > >>should it be disallowed?
> > >
> > >Opinions on this appear to vary.  A Wikipedia article defines weights
> > >as non-negative, while a manual to which it refers only uses non-zero.
> > >
> > >https://en.wikipedia.org/wiki/Weighted_arithmetic_mean#Mathematical_definition
> > >https://www.gnu.org/software/gsl/manual/html_node/Weighted-Samples.html
> > 
> > I don't think that actually allows negative weights. It says that
> > 
> >w_i = 1/\sigma_i^2
> > 
> > and variance is always > 0, thus w_i > 0. The zero is used as a special flag
> > to remove the sample from the data set in a simple way.
> > 
> > >I'm not sure which if either would be authoritative, but I could
> > >certainly make up variants for each assumption.
> > >
> > >The assumption they have in common about weights is that a zero
> > >weight is not part of the calculation, which assumption is
> > >implemented in the previously submitted code.
> > 
> > I think that if we're not sure what should happen with negative weights,
> > then we should disallow them. It's easy to allow them later once we have a
> > reasonable definition, but if we allow them now and later realize it should
> > behave differently, we'll be in trouble because of breaking existing uses.
> 
> OK
> 
> > I can't really come up with a reasonable example that would actually use
> > negative weights. Can you? That would probably help with defining the
> > behavior correctly.
> 
> No, but I'm not a statistician.  I've seen them mentioned in contexts
> that appear to be discussions among same, and again opinions vary.
> 
> > Allowing negative weights has other consequences. For example, what if
> > sum(W) ends up being 0? For example
> > 
> > CREATE TABLE t (a float, b float);
> > INSERT INTO  t SELECT i,  1 FROM generate_series(1,1000) s(i);
> > INSERT INTO  t SELECT i, -1 FROM generate_series(1,1000) s(i);
> > 
> > SELECT weighted_avg(a,b) FROM t;
> >  weighted_avg
> > --
> >   NaN
> > (1 row)
> > 
> > Is that the correct behavior? Why?
> 
> It's not, and you're right.
> 
> I will send a patch that disallows negative weights this evening or
> tomorrow.  It will be slightly more complicated as I believe I will
> need to create a new accumulator function for the weighted_avg() case
> where I had been using an extant one before.
> 
> Cheers,
> David.

Sorry about the delay.  This patch disallows negative weights,
although it still has that odd difference Jeff found.

Cheers,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 88145c5..b939340 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -12750,6 +12750,29 @@ NULL baz(3 rows)
  
   

+weighted_average
+   
+   
+weighted_avg
+   
+   weighted_avg(value 
expression, weight 
expression)
+  
+  
+   smallint, int,
+   bigint, real, double
+   precision, numeric, or interval
+  
+  
+   numeric for any integer-type argument,
+   double precision for a floating-point argument,
+   otherwise the same as the argument data type
+  
+  the average (arithmetic mean) of all input values, weighted by 
the input weights
+ 
+
+ 
+  
+   
 bit_and

bit_and(expression)
@@ -13430,6 +13453,29 @@ SELECT xmlagg(x) FROM (SELECT x FROM test ORDER BY y 
DESC) AS tab;
  
   

+weighted standard deviation
+population
+   
+   
+weighted_stddev_pop
+   
+   weighted_stddev_pop(value 
expression, weight 
expression)
+  
+  
+   smallint, int,
+   bigint, real, double
+   precision, or numeric
+  
+  
+   double precision for floating-point arguments,
+   otherwise numeric
+  
+  weighted population standard deviation of the input values
+ 
+
+ 
+  
+   
 standard deviation
 sample

@@ -13454,6 +13500,29 @@ SELECT xmlagg(x) FROM (SELECT x FROM test ORDER BY y 
DESC) AS tab;
  
   

+weighted standard deviation
+sample
+   
+   
+weighted_stddev_samp
+   
+   weighted_stddev_samp(value 
expression, weight 
expression)
+  
+  
+   

Re: [HACKERS] pg_hba_lookup function to get all matching pg_hba.conf entries

[Tom Lane]

Robert Haas  writes:
> On Fri, Apr 8, 2016 at 3:24 PM, David Steele  wrote:
>> Can one of the reviewers decide if this is ready to commit?  I fear it
>> will be pushed to the next CF otherwise.  I don't think the committers
>> have time to make that determination today...

> Well, it's not getting committed unless some committer determines that
> it is ready to commit.

I took a quick look; IMO it is certainly not ready to commit.

* Why is the IP address parameter declared as "text" and not "inet"?
Why is it optional --- it doesn't seem to me that it can have a useful
default value?

* Docs claim that ssl_inuse is of type text, when it's really bool,
and that the result is record when it's really setof record.

* While I agree that allowing ssl_inuse to default to false is probably
okay, I wonder how well this function signature will cope if we ever add
more things that pg_hba matching depends on.

* The patch seems mighty invasive to the auth code, which is not really
a place where we want a lot of churn and opportunity for mistakes.  Is
there another way to do this?  Do we really *need* a line-by-line report
of why specific lines didn't match?

* I'm a tad suspicious of the memory management, in particular the
random-looking rearrangement of where PostmasterContext gets created
and deleted.  It'd likely be better to fix things so that load_hba
doesn't have a hard-wired reference to PostmasterContext in the first
place, but is told which context to allocate under.


More generally, I'm not convinced about the use-case for this patch.
What problem is it supposed to help in dealing with, exactly?  Not syntax
errors in the hba file, evidently, since it doesn't make any attempt to
instrument the file parser.  And it's not very clear that it'll help
with "I can't connect", either, because if you can't connect you're
not going to be running this function.

If people actually need more help in figuring out why the hba line matcher
selected the line it did, I'm inclined to think maybe what's needed is a
logging option that would print a verbose trace of match/no-match
decisions.  More or less the same data this returns, really, but directed
to the postmaster log.  That way you'd be able to see it even when you're
not getting let into the database.

On the whole, though, I wonder if we shouldn't just tweak log_connections
so that it records which pg_hba line was matched.  (We already have
logging about which line was matched on an auth failure.)  I'm not really
convinced that a SQL function like this is going to be very helpful.

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] snapshot too old, configured by time

[Peter Geoghegan]

On Fri, Apr 8, 2016 at 12:55 PM, Kevin Grittner  wrote:
> Sadly, I forgot to include the reviewer information when writing the
> commit messages.  :-(

Oh well. I'm just glad we got the patch over the line. I think that
there are some types of users that will very significantly benefit
from this patch.

I am reminded of this blog post, written by a friend and former
co-worker: https://brandur.org/postgres-queues


-- 
Peter Geoghegan


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

Re: [HACKERS] snapshot too old, configured by time

[Kevin Grittner]

On Thu, Apr 7, 2016 at 6:12 PM, Peter Geoghegan  wrote:

> I think that there is a good argument in favor of this patch that you
> may have failed to make yourself, which is: it limits bloat in a way
> that's analogous to how RecentGlobalDataXmin can do so for logical
> decoding (i.e. where wal_level = logical, and RecentGlobalXmin and
> RecentGlobalDataXmin could actually differ). Therefore, it benefits to
> a significant degree from the testing that Andres did to make sure
> logical decoding doesn't cause excessive bloat when RecentGlobalXmin
> is pinned to make historic MVCC catalog snapshots work (he did so at
> my insistence at the time; pruning turned out to be very important for
> many common workloads, and Andres got that right). I can't really
> imagine a way that what you have here could be any less effective than
> what Andres did for logical decoding. This is reassuring, since that
> mechanism has to be pretty well battle-hardened by now.

Interesting.  I had not noticed that relationship.

Anyway, pushed as two patches -- the no-op patch to create the "forced
choice" on whether to do the test at each BufferGetPage point, and the
actual feature.

Sadly, I forgot to include the reviewer information when writing the
commit messages.  :-(

-- 
Kevin Grittner
EDB: 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] [COMMITTERS] pgsql: CREATE INDEX ... INCLUDING (column[, ...])

[Stephen Frost]

Anastasia,

Attached is the patch to fix pg_dump against older versions, which was
broken in the committed patch.

Thanks!

Stephen
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
new file mode 100644
index 7e6abd7..7c5ae31
*** a/src/bin/pg_dump/pg_dump.c
--- b/src/bin/pg_dump/pg_dump.c
*** getIndexes(Archive *fout, TableInfo tbli
*** 6088,6093 
--- 6088,6095 
  			  "SELECT t.tableoid, t.oid, "
  			  "t.relname AS indexname, "
  	 "pg_catalog.pg_get_indexdef(i.indexrelid) AS indexdef, "
+ 			  "NULL AS indnkeyatts, "
+ 			  "NULL AS indnatts, "
  			  "t.relnatts AS indnkeys, "
  			  "i.indkey, i.indisclustered, "
  			  "i.indisreplident, t.relpages, "
*** getIndexes(Archive *fout, TableInfo tbli
*** 6119,6124 
--- 6121,6128 
  			  "SELECT t.tableoid, t.oid, "
  			  "t.relname AS indexname, "
  	 "pg_catalog.pg_get_indexdef(i.indexrelid) AS indexdef, "
+ 			  "NULL AS indnkeyatts, "
+ 			  "NULL AS indnatts, "
  			  "t.relnatts AS indnkeys, "
  			  "i.indkey, i.indisclustered, "
  			  "false AS indisreplident, t.relpages, "
*** getIndexes(Archive *fout, TableInfo tbli
*** 6146,6151 
--- 6150,6157 
  			  "SELECT t.tableoid, t.oid, "
  			  "t.relname AS indexname, "
  	 "pg_catalog.pg_get_indexdef(i.indexrelid) AS indexdef, "
+ 			  "NULL AS indnkeyatts, "
+ 			  "NULL AS indnatts, "
  			  "t.relnatts AS indnkeys, "
  			  "i.indkey, i.indisclustered, "
  			  "false AS indisreplident, t.relpages, "
*** getIndexes(Archive *fout, TableInfo tbli
*** 6176,6181 
--- 6182,6189 
  			  "SELECT t.tableoid, t.oid, "
  			  "t.relname AS indexname, "
  	 "pg_catalog.pg_get_indexdef(i.indexrelid) AS indexdef, "
+ 			  "NULL AS indnkeyatts, "
+ 			  "NULL AS indnatts, "
  			  "t.relnatts AS indnkeys, "
  			  "i.indkey, i.indisclustered, "
  			  "false AS indisreplident, t.relpages, "
*** getIndexes(Archive *fout, TableInfo tbli
*** 6205,6210 
--- 6213,6220 
  			  "SELECT t.tableoid, t.oid, "
  			  "t.relname AS indexname, "
  	 "pg_catalog.pg_get_indexdef(i.indexrelid) AS indexdef, "
+ 			  "NULL AS indnkeyatts, "
+ 			  "NULL AS indnatts, "
  			  "t.relnatts AS indnkeys, "
  			  "i.indkey, i.indisclustered, "
  			  "false AS indisreplident, t.relpages, "
*** getIndexes(Archive *fout, TableInfo tbli
*** 6234,6239 
--- 6244,6251 
  			  "SELECT t.tableoid, t.oid, "
  			  "t.relname AS indexname, "
  			  "pg_get_indexdef(i.indexrelid) AS indexdef, "
+ 			  "NULL AS indnkeyatts, "
+ 			  "NULL AS indnatts, "
  			  "t.relnatts AS indnkeys, "
  			  "i.indkey, false AS indisclustered, "
  			  "false AS indisreplident, t.relpages, "
*** getIndexes(Archive *fout, TableInfo tbli
*** 6261,6266 
--- 6273,6280 
  			  "t.oid, "
  			  "t.relname AS indexname, "
  			  "pg_get_indexdef(i.indexrelid) AS indexdef, "
+ 			  "NULL AS indnkeyatts, "
+ 			  "NULL AS indnatts, "
  			  "t.relnatts AS indnkeys, "
  			  "i.indkey, false AS indisclustered, "
  			  "false AS indisreplident, t.relpages, "


signature.asc
Description: Digital signature

Re: [HACKERS] [patch] Proposal for \crosstabview in psql

[Alvaro Herrera]

Robert Haas wrote:

> This seems like it might be converging on some sort of consensus, but
> I'm wondering if we shouldn't push it to 9.7, instead of rushing
> decisions that we will later have trouble changing on
> backward-compatibility grounds.

My intention is to commit this afternoon in the next couple of hours,
and only the most basic case is going to be supported, and the rest of
the cases (concatenation of several fields and several rows, etc) are
just going to throw errors; that way, it will be easy to add more
features later as they are agreed upon.

-- 
Álvaro Herrerahttp://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] [patch] Proposal for \crosstabview in psql

[Robert Haas]

On Fri, Apr 8, 2016 at 7:23 AM, Daniel Verite  wrote:
> Alvaro Herrera wrote:
>
>> I wonder if the business of appending values of multiple columns
>> separated with spaces is doing us any good.  Why not require that
>> there's a single column in the cell?  If the user wants to put things
>> together, they can use format() or just || the fields together.  What
>> benefit is there to the ' '?  When I ran my first test queries over
>> pg_class I was surprised about this behavior:
>> alvherre=# select * from pg_class
>> alvherre=# \crosstabview relnatts relkind
>
> ISTM that this could be avoided by erroring out for lack of an
> explicit 3rd column as argument. IOW, we wouldn't assume
> that "no column specified" means "show all columns".
>
> About simply ripping out the possibility of having multiple
> columns into cells, it's more radical but if that part turns out to
> be more confusing than useful, I don't have a problem
> with removing it.
>
> The other case of stringing multiple contents into the same cell
> is when different tuples carry (row,column) duplicates.
> I'm not inclined to disallow that case, I think it would go too far
> in guessing what the user expects.
> My expectation for a viewer is that it displays the results as far as
> possible, whatever they are.
> Also, showing such contents in vertically-growing cells as it
> does now allows the user to spot these easily in the grid when
> they happen to be outliers. I'm seeing it as useful in that case.

This seems like it might be converging on some sort of consensus, but
I'm wondering if we shouldn't push it to 9.7, instead of rushing
decisions that we will later have trouble changing on
backward-compatibility grounds.

-- 
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] pg_hba_lookup function to get all matching pg_hba.conf entries

[Robert Haas]

On Fri, Apr 8, 2016 at 3:24 PM, David Steele  wrote:
> On 4/5/16 9:52 PM, Robert Haas wrote:
>> On Mon, Mar 21, 2016 at 3:31 AM, Haribabu Kommi
>>  wrote:
>>> On Mon, Mar 21, 2016 at 2:00 PM, Alvaro Herrera
>>>  wrote:
 Haribabu Kommi wrote:

>> Check.
>>
>> +} lookup_hba_line_context;
>> ^ but why TAB here?
>
> corrected. I am not sure why pg_indent is adding a tab here.

 It's because lookup_hba_line_context is not listed in typedefs.list.
 I suggest adding it and all other new typedefs you add, and rerunning
 pgindent, as the lack of those may affect other places where those names
 appear.
>>>
>>> Thanks for the details. I added the new typedef into typedefs.list file.
>>> Updated patch is attached.
>>
>> This patch is still marked "needs review".  If it's ready to go, one
>> of the reviewers should mark it "ready for committer".
>
> Can one of the reviewers decide if this is ready to commit?  I fear it
> will be pushed to the next CF otherwise.  I don't think the committers
> have time to make that determination today...

Well, it's not getting committed unless some committer determines that
it is ready to commit.  As far as 9.6 goes, that committer will not be
me; my commit bit is starting to smoke, and anything I try to do in
the next few hours is likely to have an unacceptable error rate.  I am
beat.  But I think we have a good release to look forward to.

-- 
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] multivariate statistics v14

[Robert Haas]

On Fri, Apr 8, 2016 at 3:13 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> On Fri, Apr 8, 2016 at 2:55 PM, Tomas Vondra
>>  wrote:
>>> Well, me to. But my feeling is the patch received entirely insufficient
>>> amount of thorough code review, considering how important part of the code
>>> it touches. I agree docs are an important part of a patch, but polishing
>>> user-level docs would hardly move the patch closer to being committable
>>> (especially when there's ~50kB of READMEs).
>
>> I have to admit that I was really hoping Tom would follow through on
>> his statement that he would look into this one, or that Dean Rasheed
>> would get involved.
>
> I'm sorry I didn't get to it, but it's not like I have been slacking
> during this commitfest.  At some point, you just have to accept that
> not everything we could wish will get into 9.6.

I did not mean to imply otherwise.  I'm just explaining why I didn't
spend time on it - I figured I was not the most qualified person, and
of course I have not been slacking either.  :-)

> I will make it a high priority for 9.7, though.

Woohoo!

-- 
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: pg_hba_lookup function to get all matching pg_hba.conf entries

[David Steele]

On 4/5/16 9:52 PM, Robert Haas wrote:

> On Mon, Mar 21, 2016 at 3:31 AM, Haribabu Kommi
>  wrote:
>> On Mon, Mar 21, 2016 at 2:00 PM, Alvaro Herrera
>>  wrote:
>>> Haribabu Kommi wrote:
>>>
> Check.
>
> +} lookup_hba_line_context;
> ^ but why TAB here?

 corrected. I am not sure why pg_indent is adding a tab here.
>>>
>>> It's because lookup_hba_line_context is not listed in typedefs.list.
>>> I suggest adding it and all other new typedefs you add, and rerunning
>>> pgindent, as the lack of those may affect other places where those names
>>> appear.
>>
>> Thanks for the details. I added the new typedef into typedefs.list file.
>> Updated patch is attached.
> 
> This patch is still marked "needs review".  If it's ready to go, one
> of the reviewers should mark it "ready for committer".

Can one of the reviewers decide if this is ready to commit?  I fear it
will be pushed to the next CF otherwise.  I don't think the committers
have time to make that determination today...

-- 
-David
da...@pgmasters.net


-- 
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] 2016-03 Commitfest

[Alexander Korotkov]

On Fri, Apr 8, 2016 at 6:52 PM, Robert Haas  wrote:

> On Fri, Apr 8, 2016 at 11:00 AM, Andres Freund  wrote:
> > On 2016-04-08 10:56:28 -0400, Robert Haas wrote:
> >> On Fri, Apr 8, 2016 at 10:06 AM, David Steele 
> wrote:
> >> > On 4/8/16 10:00 AM, Tom Lane wrote:
> >> >> David Steele  writes:
> >> >>> So the commitfest is 84% complete with less than twelve hours to go.
> >> >>
> >> >> Have we set a particular time-of-day for closing the CF, and if so
> >> >> what is it exactly?
> >> >
> >> > From the referenced email:
> >> >
> >> > "Accordingly, the release management has decided that all
> >> > feature patches destined for PostgreSQL 9.6 must be committed no later
> >> > than April 8, 2016.  Any patch not committed prior to 2016-04-09
> >> > 00:00:00 GMT may not be committed to PostgreSQL 9.6 unless (a) it is a
> >> > bug fix, (b) it represents essential cleanup of a previously-committed
> >> > patch, or (c) the release management team has approved an extension to
> >> > the deadline for that particular patch."
> >>
> >> IOW, the deadline is 8pm US/Eastern time, or about 9 hours from now.
> >>
> >> Let's try not to introduce more bugs in the next 9 hours than we have
> >> in the preceding 9 months.
> >
> > I've finished polishing the Pin/Unpin patch. But the final polishing
> > happened on an intercontential flight, after days spent preparing my
> > move to SF. I'd be glad if you would allow me to look over the patch
> > again, before pushing it sometime this weekend; this stuff is subtle,
> > and I'm not exactly my best right now.
>
> In view of these circumstances, the RMT has voted to extend the
> deadline for this particular patch by 2.5 days; that is, this patch
> may be committed with RMT approval no later than 2016-04-11 12:00:00
> GMT, which I believe is approximately 4am Monday morning where you
> are.
>

Good to hear.  Wise decision, because we really need this patch in 9.6.

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

Re: [HACKERS] Move PinBuffer and UnpinBuffer to atomics

[Alexander Korotkov]

On Fri, Apr 8, 2016 at 7:39 PM, Andres Freund  wrote:

> On 2016-04-07 16:50:44 +0300, Alexander Korotkov wrote:
> > On Thu, Apr 7, 2016 at 4:41 PM, Andres Freund 
> wrote:
> >
> > > On 2016-03-31 20:21:02 +0300, Alexander Korotkov wrote:
> > > > ! BEGIN_BUFSTATE_CAS_LOOP(bufHdr);
> > > >
> > > > ! Assert(BUF_STATE_GET_REFCOUNT(state) > 0);
> > > > ! wasDirty = (state & BM_DIRTY) ? true : false;
> > > > ! state |= BM_DIRTY | BM_JUST_DIRTIED;
> > > > ! if (state == oldstate)
> > > > ! break;
> > >
> > > I'm doubtful that this early exit is entirely safe. None of the
> > > preceding operations imply a memory barrier. The buffer could
> previously
> > > have been marked dirty, but cleaned since. It's pretty critical that we
> > > re-set the dirty bit (there's no danger of loosing it with a barrier,
> > > because we hold an exclusive content lock).
> > >
> >
> > Oh, I get it.
> >
> >
> > > Practically the risk seems fairly low, because acquiring the exclusive
> > > content lock will have implied a barrier. But it seems unlikely to have
> > > a measurable performance effect to me, so I'd rather not add the early
> > > exit.
> > >
> >
> > Ok, let's just remove it.
>
> Here's my updated version of the patch. I've updated this on an
> intercontinental flight, after a otherwise hectic week (moving from SF
> to Berlin); so I'm planning to look over this once more before pushing (.
>

Ok.

I've decided that the cas-loop macros are too obfuscating for my
> taste. To avoid duplicating the wait part I've introduced
> WaitBufHdrUnlocked().
>

That's OK for me.  Cas-loop macros looks cute, but too magic.


> As you can see in
>
> http://archives.postgresql.org/message-id/CA%2BTgmoaeRbN%3DZ4oWENLvgGLeHEvGZ_S_Z3KGrdScyKiSvNt3oA%40mail.gmail.com
> I'm planning to apply this sometime this weekend, after running some
> tests and going over the patch again.
>
> Any chance you could have a look over this?
>

I took a look at this.  Changes you made look good for me.
I also run test on 4x18 Intel server.

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

Re: [HACKERS] multivariate statistics v14

[Tom Lane]

Robert Haas  writes:
> On Fri, Apr 8, 2016 at 2:55 PM, Tomas Vondra
>  wrote:
>> Well, me to. But my feeling is the patch received entirely insufficient
>> amount of thorough code review, considering how important part of the code
>> it touches. I agree docs are an important part of a patch, but polishing
>> user-level docs would hardly move the patch closer to being committable
>> (especially when there's ~50kB of READMEs).

> I have to admit that I was really hoping Tom would follow through on
> his statement that he would look into this one, or that Dean Rasheed
> would get involved.

I'm sorry I didn't get to it, but it's not like I have been slacking
during this commitfest.  At some point, you just have to accept that
not everything we could wish will get into 9.6.

I will make it a high priority for 9.7, though.

regards, tom lane


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

Re: [HACKERS] raw output from copy

[Pavel Stehule]

2016-04-08 20:54 GMT+02:00 Andrew Dunstan :

>
>
> On 04/08/2016 02:13 PM, Robert Haas wrote:
>
>> On Tue, Apr 5, 2016 at 4:45 AM, Pavel Stehule 
>> wrote:
>>
>>> here is cleaned/finished previous implementation of RAW_TEXT/RAW_BINARY
>>> formats for COPY statements.
>>>
>>> The RAW with text formats means unescaped data, but with correct
>>> encoding -
>>> input/output is realised with input/output function. RAW binary means
>>> content produced/received by sending/received functions.
>>>
>>> Now both directions (input/output) working well
>>>
>>> Some examples of expected usage:
>>>
>>> copy (select xmlelement(name foo, 'hello')) to stdout (format raw_binary,
>>> encoding 'latin2');
>>>
>>> create table avatars(id serial, picture bytea);
>>> \copy avatars(picture) from ~/images/foo.jpg (format raw_binary);
>>> select lastval();
>>>
>>> create table doc(id serial, txt text);
>>> \copy doc(txt) from ~/files/aaa.txt (format raw_text, encoding 'latin2');
>>> select lastval();
>>>
>> As much as I know you and some other people would like it to be
>> otherwise, this patch clearly does not have a sufficient degree of
>> consensus to justify committing it to PostgreSQL 9.6.  I'm marking it
>> Returned with Feedback.
>>
>>
>
> I should add that I've been thinking about this some more, and that I now
> agree that something should be done to support this at the SQL level,
> mainly so that clients can manage very large pieces of data in a
> stream-oriented fashion rather than having to marshall the data in memory
> to load/unload via INSERT/SELECT. Anything that is client-side only is
> likely to have this memory issue.
>
> At the same time I'm still not entirely convinced that COPY is a good
> vehicle for this. It's designed for bulk records, and already quite
> complex. Maybe we need something new that uses the COPY protocol but is
> more specifically tailored for loading or sending large singleton pieces of
> data.
>

Now it is little bit more time to think more about. But It is hard to
design some more simpler than is COPY syntax. What will support both
directions.

My implementation has same limit like COPY BINARY - it isn't worse. It
should be good enough for VARLENA types that should not be higher than 1GB.
It is not designed for LOB replacement.

Regards

Pavel


>
> cheers
>
> andrew
>
>
>
>

Re: [HACKERS] PoC: Partial sort

[Peter Geoghegan]

On Wed, Mar 30, 2016 at 8:02 AM, Alexander Korotkov
 wrote:
> Hmm... I'm not completely agree with that. In typical usage partial sort
> should definitely use quicksort.  However, fallback to other sort methods is
> very useful.  Decision of partial sort usage is made by planner.  But
> planner makes mistakes.  For example, our HashAggregate is purely in-memory.
> In the case of planner mistake it causes OOM.  I met such situation in
> production and not once.  This is why I'd like partial sort to have graceful
> degradation for such cases.

I think that this should be moved to the next CF, unless a committer
wants to pick it up today.


-- 
Peter Geoghegan


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

Fwd: [HACKERS] Speedup twophase transactions

[Stas Kelvich]

> On 08 Apr 2016, at 21:55, Jesper Pedersen  wrote:
> 
> On 04/08/2016 02:42 PM, Robert Haas wrote:
>> On Tue, Jan 26, 2016 at 7:43 AM, Stas Kelvich  
>> wrote:
>>> Hi,
>>> 
>>> Thanks for reviews and commit!
>> 
>> I apologize for being clueless here, but was this patch committed?
>> It's still marked as "Needs Review" in the CommitFest application.
>> 
> 
> There are 2 parts to this - both in the same email thread.
> 
> Part 1 [0] dealt with 2-phase commits on the master node. Part 2 [1] deals 
> with replaying on slaves, which currently shows lag.
> 
> There is still an open item found by Michael, so part 2 isn't ready to be 
> moved to "Ready for Committer" yet.
> 
> [0] 
> http://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=978b2f65aa1262eb4ecbf8b3785cb1b9cf4db78e
> [1] https://commitfest.postgresql.org/9/523/
> 
> Best regards,
> Jesper
> 

By the way, Jesper, can you, please, try to run tests in diff, that i’ve sent 
today? It includes test scenario that was
causing problems for Michael, but works fine on all systems that I have access 
to.


Stas Kelvich
Postgres Professional: http://www.postgrespro.com
The Russian Postgres 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] multivariate statistics v14

[Robert Haas]

On Fri, Apr 8, 2016 at 2:55 PM, Tomas Vondra
 wrote:
> Well, me to. But my feeling is the patch received entirely insufficient
> amount of thorough code review, considering how important part of the code
> it touches. I agree docs are an important part of a patch, but polishing
> user-level docs would hardly move the patch closer to being committable
> (especially when there's ~50kB of READMEs).

I have to admit that I was really hoping Tom would follow through on
his statement that he would look into this one, or that Dean Rasheed
would get involved.  I am sure I could do a good review of this patch
given enough time, but I am also sure that it would take an amount of
time that is at least one if not two orders of magnitude more than I
put into any patch this CommitFest.  I understand statistics at some
basic level, but I am not an expert on them the way some people here
are.

-- 
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] Speedup twophase transactions

[Stas Kelvich]

> On 08 Apr 2016, at 21:42, Robert Haas  wrote:
> 
> On Tue, Jan 26, 2016 at 7:43 AM, Stas Kelvich  
> wrote:
>> Hi,
>> 
>> Thanks for reviews and commit!
> 
> I apologize for being clueless here, but was this patch committed?
> It's still marked as "Needs Review" in the CommitFest application.

There was a patch to skip two phase file creation when there were no checkpoint
between PREPARE and COMMIT, and that patch was commited.
But that patch didn’t touch anything in replay, so replay speed of 2pc is 
significantly slower
than 2pc in normal mode. And that can cause constantly increasing replication 
lag for async
replication.
After that i’ve wrote new patch introducing same behaviour in replay and used 
the same
mail thread. Now Michael found a (heisen)bug in second patch, that i can’t 
reproduce.

Stas Kelvich
Postgres Professional: http://www.postgrespro.com
The Russian Postgres 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] Speedup twophase transactions

[Jesper Pedersen]

On 04/08/2016 02:37 PM, Robert Haas wrote:

On Fri, Apr 8, 2016 at 8:49 AM, Jesper Pedersen
 wrote:

On 04/07/2016 02:29 AM, Michael Paquier wrote:

So recovery is conflicting here. My guess is that this patch is
missing some lock cleanup.

With the test case attached in my case the COMMIT PREPARED record does
not even get replayed.



Should we create an entry for the open item list [0] for this, due to the
replication lag [1] ?

CommitFest entry [2]
Original commit [3]

Cc'ed RMT.


If there is something you think needs to be fixed that is a new issue
in 9.6, then yes you should.  I don't quite understand what thing is
from reading this, so please make sure to describe it clearly.



Michael, you seem to have the necessary permission for this. Could you 
add an entry ?


Best regards,
 Jesper



--
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] proposal: PL/Pythonu - function ereport

[Pavel Stehule]

2016-04-08 20:52 GMT+02:00 Tom Lane :

> Pavel Stehule  writes:
> > 2016-04-08 17:38 GMT+02:00 Teodor Sigaev :
> >> thank you, pushed. Pls, pay attention to buildfarm.
>
> > Thank you very much for commit.
>
> According to buildfarm member prairiedog, there's a problem in one
> of the test cases.  I suspect that it's using syntax that doesn't
> exist in Python 2.3, but don't know enough Python to fix it.
>
> Please submit a correction -- we are not moving the goalposts on
> Python version compatibility for the convenience of one test case.
>

I'll fix it.

Regards

Pavel

>
> regards, tom lane
>

Re: [HACKERS] raw output from copy

[Andrew Dunstan]

On 04/08/2016 02:13 PM, Robert Haas wrote:

On Tue, Apr 5, 2016 at 4:45 AM, Pavel Stehule  wrote:

here is cleaned/finished previous implementation of RAW_TEXT/RAW_BINARY
formats for COPY statements.

The RAW with text formats means unescaped data, but with correct encoding -
input/output is realised with input/output function. RAW binary means
content produced/received by sending/received functions.

Now both directions (input/output) working well

Some examples of expected usage:

copy (select xmlelement(name foo, 'hello')) to stdout (format raw_binary,
encoding 'latin2');

create table avatars(id serial, picture bytea);
\copy avatars(picture) from ~/images/foo.jpg (format raw_binary);
select lastval();

create table doc(id serial, txt text);
\copy doc(txt) from ~/files/aaa.txt (format raw_text, encoding 'latin2');
select lastval();

As much as I know you and some other people would like it to be
otherwise, this patch clearly does not have a sufficient degree of
consensus to justify committing it to PostgreSQL 9.6.  I'm marking it
Returned with Feedback.




I should add that I've been thinking about this some more, and that I 
now agree that something should be done to support this at the SQL 
level, mainly so that clients can manage very large pieces of data in a 
stream-oriented fashion rather than having to marshall the data in 
memory to load/unload via INSERT/SELECT. Anything that is client-side 
only is likely to have this memory issue.


At the same time I'm still not entirely convinced that COPY is a good 
vehicle for this. It's designed for bulk records, and already quite 
complex. Maybe we need something new that uses the COPY protocol but is 
more specifically tailored for loading or sending large singleton pieces 
of data.


cheers

andrew





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

Re: [HACKERS] multivariate statistics v14

[Tomas Vondra]

On 04/08/2016 05:55 PM, Robert Haas wrote:

On Tue, Mar 29, 2016 at 11:18 AM, David Steele  wrote:

On 3/28/16 4:42 AM, Tomas Vondra wrote:

Yes, those are valid omissions. I plan to address them, and I'd also
considering adding a section to 65.1 (How the Planner Uses Statistics),
explaining more thoroughly how the planner uses multivariate stats.


It looks you need post a new patch so I have marked this "waiting on
author".


Since no new version of this patch has been posted in the last 10
days, it seems clear that there will not be time for this to
reasonably become ready for committer and then get committed in the
few hours remaining before the deadline. That is a bummer, since I
was hoping we would have this feature in this release, but hopefully
we will get it into 9.7. I am marking it Returned with Feedback.



Well, me to. But my feeling is the patch received entirely insufficient 
amount of thorough code review, considering how important part of the 
code it touches. I agree docs are an important part of a patch, but 
polishing user-level docs would hardly move the patch closer to being 
committable (especially when there's ~50kB of READMEs).


regards

--
Tomas Vondra  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] Speedup twophase transactions

[Jesper Pedersen]

On 04/08/2016 02:42 PM, Robert Haas wrote:

On Tue, Jan 26, 2016 at 7:43 AM, Stas Kelvich  wrote:

Hi,

Thanks for reviews and commit!


I apologize for being clueless here, but was this patch committed?
It's still marked as "Needs Review" in the CommitFest application.



There are 2 parts to this - both in the same email thread.

Part 1 [0] dealt with 2-phase commits on the master node. Part 2 [1] 
deals with replaying on slaves, which currently shows lag.


There is still an open item found by Michael, so part 2 isn't ready to 
be moved to "Ready for Committer" yet.


[0] 
http://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=978b2f65aa1262eb4ecbf8b3785cb1b9cf4db78e

[1] https://commitfest.postgresql.org/9/523/

Best regards,
 Jesper



--
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] proposal: PL/Pythonu - function ereport

[Tom Lane]

Pavel Stehule  writes:
> 2016-04-08 17:38 GMT+02:00 Teodor Sigaev :
>> thank you, pushed. Pls, pay attention to buildfarm.

> Thank you very much for commit.

According to buildfarm member prairiedog, there's a problem in one
of the test cases.  I suspect that it's using syntax that doesn't
exist in Python 2.3, but don't know enough Python to fix it.

Please submit a correction -- we are not moving the goalposts on
Python version compatibility for the convenience of one test case.

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] Show dropped users' backends in pg_stat_activity

[Tom Lane]

Oskari Saarenmaa  writes:
> 24.03.2016, 18:03, Tom Lane kirjoitti:
>> I concur.  Let's put the left join(s) into those views and call it
>> good.

> Added my patch to the 2016-09 commitfest 
> (https://commitfest.postgresql.org/10/601/) as a bug fix as I thought 
> not showing all backends in pg_stat_activity is a bug.  Any chance to 
> get it in 9.6?

I do not think this is a sufficiently high-priority bug to justify
putting it into 9.6 at this point.  It's been like this for years and
there's a noticeable lack of any field complaints.  Fixing it in the
next release cycle seems fine to me.

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] Fix for OpenSSL error queue bug

[Peter Geoghegan]

On Fri, Apr 8, 2016 at 11:42 AM, Tom Lane  wrote:
> Seems like a reasonable thing to do, but somebody would have to do the
> legwork to produce back-branch patches.

I'll do so soon. I was waiting on Peter E to take me up on the offer.

-- 
Peter Geoghegan


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

Re: [HACKERS] Fix for OpenSSL error queue bug

[Tom Lane]

Peter Geoghegan  writes:
> On Fri, Apr 8, 2016 at 11:20 AM, Peter Geoghegan  wrote:
>> That seems reasonable. I'm glad we finally got this done. Thanks.

> Are we going to backpatch this?

Seems like a reasonable thing to do, but somebody would have to do the
legwork to produce back-branch patches.

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] Speedup twophase transactions

[Robert Haas]

On Tue, Jan 26, 2016 at 7:43 AM, Stas Kelvich  wrote:
> Hi,
>
> Thanks for reviews and commit!

I apologize for being clueless here, but was this patch committed?
It's still marked as "Needs Review" in the CommitFest application.

-- 
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 v12] GSSAPI encryption support

[Robert Haas]

On Fri, Apr 8, 2016 at 2:36 PM, Stephen Frost  wrote:
> While it seems like this particular patch (with myself as committer)
> would meet the requirements stated by the RMT for an extension, having
> considered it over the past day or so, I don't think we should make it a
> policy to allow an extension when it involves a significant rework of
> the patch, as is the case here.

I agree.  To be clear, those were intended as necessary but not
necessarily sufficient reasons for extension.  I agree that patches
needing significant reworking are not good candidates for extensions.
(But that is my feeling as an RMT member, not an RMT official policy
upon which we have voted.)

-- 
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] proposal: PL/Pythonu - function ereport

[Pavel Stehule]

2016-04-08 17:38 GMT+02:00 Teodor Sigaev :

> Thank you very much
>>
>
> thank you, pushed. Pls, pay attention to buildfarm.
>

Thank you very much for commit.

And big thanks to Iacob for big help.

Regards

Pavel



>
> --
> Teodor Sigaev   E-mail: teo...@sigaev.ru
>WWW:
> http://www.sigaev.ru/
>

Re: [HACKERS] Speedup twophase transactions

[Robert Haas]

On Fri, Apr 8, 2016 at 8:49 AM, Jesper Pedersen
 wrote:
> On 04/07/2016 02:29 AM, Michael Paquier wrote:
>> So recovery is conflicting here. My guess is that this patch is
>> missing some lock cleanup.
>>
>> With the test case attached in my case the COMMIT PREPARED record does
>> not even get replayed.
>>
>
> Should we create an entry for the open item list [0] for this, due to the
> replication lag [1] ?
>
> CommitFest entry [2]
> Original commit [3]
>
> Cc'ed RMT.

If there is something you think needs to be fixed that is a new issue
in 9.6, then yes you should.  I don't quite understand what thing is
from reading this, so please make sure to describe it clearly.

-- 
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 v12] GSSAPI encryption support

[Stephen Frost]

* Robert Haas (robertmh...@gmail.com) wrote:
> On Thu, Apr 7, 2016 at 10:17 PM, Michael Paquier
>  wrote:
> > On Thu, Apr 7, 2016 at 8:20 AM, Tom Lane  wrote:
> >> Robbie Harwood  writes:
> >>> Tom Lane  writes:
>  Wait a second.  So the initial connection-request packet is necessarily
>  unencrypted under this scheme?
> >>
> >>> Yes, by necessity.  The username must be sent in the clear, even if only
> >>> as part of the GSSAPI handshake (i.e., the GSSAPI username will appear
> >>> in plantext in the GSSAPI blobs which are otherwise encrypted).  GSSAPI
> >>> performs authentication before it can start encryption.
> >>
> >> Ugh.  I had thought we were putting work into this because it represented
> >> something we could recommend as best practice, but now you're telling me
> >> that it's always going to be inferior to what we have already.
> >
> > It does not seem necessary to have an equivalent of
> > pqsecure_open_client, just some extra handling in fe-connect.c to set
> > up the initial context with a proper message handling... Not that
> > direct anyway. So should the patch be marked as returned with feedback
> > at this stage?
> 
> Yeah, I think so.  It doesn't seem we have consensus on this, and it's
> too late to be trying to build one now.

Actually, I chatted with Robbie quite a bit over IRC and he's agreed on
reworking this to use the same approach that we use for SSL, but that's
expected to take the better part of a week to do.

While it seems like this particular patch (with myself as committer)
would meet the requirements stated by the RMT for an extension, having
considered it over the past day or so, I don't think we should make it a
policy to allow an extension when it involves a significant rework of
the patch, as is the case here.

Robbie, please be sure to add this to the next commitfest and please
hound me to review it, you know where to find me. :)

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] Fix for OpenSSL error queue bug

[Peter Geoghegan]

On Fri, Apr 8, 2016 at 11:20 AM, Peter Geoghegan  wrote:
> That seems reasonable. I'm glad we finally got this done. Thanks.

Are we going to backpatch this?


-- 
Peter Geoghegan


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

Re: [HACKERS] WIP: Detecting SSI conflicts before reporting constraint violations

[Robert Haas]

On Thu, Apr 7, 2016 at 12:20 PM, Kevin Grittner  wrote:
> On Thu, Apr 7, 2016 at 8:49 AM, Thomas Munro
>  wrote:
>
>> Here is a version that includes an attempt to describe the
>> situation in the documentation.
>
> Pushed with minor adjustments to the docs.  Mostly I thought your
> new text was more appropriate as just another paragraph than as a
> "note".  The previous paragraph was a little imprecise and was in
> some conflict with the new one, so I adjusted that a little, too.
>
> Nice work!  I sure wish we had spotted that a one-line check there
> would have covered so much when the feature was first added.
>
> I understand there is considerable feeling that this should be
> back-patched, but I have not done that pending a clear consensus on
> the point, since it is a user-visible behavioral change.

I think that's a good call.  Conservatism in back-patching is entirely
warranted.

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


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

Re: [HACKERS] BUG #13854: SSPI authentication failure: wrong realm name used

[Magnus Hagander]

On Fri, Apr 8, 2016 at 1:38 PM, Magnus Hagander  wrote:

> On Tue, Mar 29, 2016 at 11:24 PM, Christian Ullrich 
> wrote:
>
>> * Magnus Hagander wrote:
>>
>> On Tue, Mar 29, 2016 at 5:09 PM, David Steele 
>>> wrote:
>>>
>>
>> It seems like this patch should be set "ready for committer".  Can one of
 the reviewers do that if appropriate?

>>>
>>> I'll pick it up to do that as well as committing it.
>>>
>>
>> Ah, good news!
>>
>> I hope it's not coming too late, but I have a final update removing a
>> rather pointless palloc() return value check. No changes otherwise.
>
>
> Small notes:
>
> * I think it's wrong to have the docs say "leave this at the default to
> maintain compatibility" in the reference section - if anything, that's for
> release notes. And it's the default behaviour. So I just removed that one
>
> * Made some other wordsmithing on the SGML.
>
> * This looks strange to me:
> if (!res || p == NULL)
>
> it's correct logically, the style just looks weird. But maybe it's a good
> idea to keep it to make it clear that res is a bool and p is a pointer. I'm
> on the fence.
>
> * it also needed a pgindent, in particular a couple of return STATUS_ERROR
> were indented in a way that made them look like they were almost in the
> wrong place, and some minor style changes. But that's all mechanical.
>
> Other than those minor things it looks good to me, so I'm going to push
> the current version with those once I'm back on reliable wifi.
>
>

And now committed. Thanks!

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

Re: [HACKERS] Proposal: BSD Authentication support

[Pierre-Emmanuel André]

On Fri, Mar 18, 2016 at 06:30:35PM +1300, Thomas Munro wrote:
> On Fri, Mar 18, 2016 at 12:49 PM, Marisa Emerson  wrote:
> > On 18/03/16 03:57, Thomas Munro wrote:
> >>
> >> You used one name in the docs and another in the code:
> >>
> >> +BSD Authentication on PostgreSQL uses the
> >> auth-postgres
> >> +login type and authenticates with the postgres
> >> login
> >>
> >> + retval = auth_userokay(user, NULL, "auth-postgresql", passwd);
> >
> >
> > Woops, fix attached.
> 
> Thanks!
> 
> I'm CCng Pierre-Emmanuel André, maintainer of the OpenBSD postgresql
> port/package, just in case he has any feedback.
> 
> Pierre-Emmanuel, here's the full thread in case you missed it:
> http://www.postgresql.org/message-id/1521c2f9465.e357a543197915.6912077634413325...@insec.sh
> 


Hi,

Sorry for the late answer.
I've tested the patch on @amd64 with the latest PostgreSQL 9.6devel. I can't 
judge the diff
itself (it looks good but i'm not a PostgreSQL developper) but everything works 
fine.
It would be a nice feature to have on OpenBSD.

Regards,



-- 
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] Show dropped users' backends in pg_stat_activity

[Oskari Saarenmaa]

24.03.2016, 18:03, Tom Lane kirjoitti:

Robert Haas  writes:

I am not really in favor of half-fixing this.  If we can't
conveniently wait until a dropped role is completely out of the
system, then I don't see a lot of point in trying to do it in the
limited cases where we can.  If LEFT JOIN is the way to go, then,
blech, but, so be it.


I concur.  Let's put the left join(s) into those views and call it
good.

BTW, I think we would need the left joins even if we had interlocking
in DROP, just to protect ourselves against race conditions.  Remember
that what pg_stat_activity shows is a snapshot, which might be more or
less out of date compared to the catalog contents.


Added my patch to the 2016-09 commitfest 
(https://commitfest.postgresql.org/10/601/) as a bug fix as I thought 
not showing all backends in pg_stat_activity is a bug.  Any chance to 
get it in 9.6?


--
Oskari Saarenmaa
Aiven: managed cloud databases
https://aiven.io


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

Re: [HACKERS] [GENERAL] pg_restore casts check constraints differently

[Victor Pontis]

Hey, I work with Josh Ma and we were troubleshooting this problem together.

We ended up creating a workaround by taking the dumps from different DBs,
initializing new DBs based on those dumps, and then dumping these new DBs.
This work around worked since the dumps of databases that were initialized
via a psql script outputted the text array constraint in the same way.

So there are definitely ways to workaround this inconsistent representation
for our use case.

Thanks again for the help!

-- 
Victor Pontis
Benchling
Engineer
858-761-5232


On Wed, Mar 30, 2016 at 9:51 AM, Amit Langote 
wrote:

> On Thu, Mar 31, 2016 at 1:00 AM, Tom Lane  wrote:
> > I wrote:
> >> Amit Langote  writes:
> >>> destdb=# ALTER TABLE c ADD CONSTRAINT p_a_check CHECK (a IN ('a', 'b',
> 'c'));
> >>> destdb=# \d c
> >>> ...
> >>> Check constraints:
> >>> "p_a_check" CHECK (a::text = ANY (ARRAY['a'::character varying,
> >>> 'b'::character varying, 'c'::character varying]::text[]))
> >
> >> Hm.  It seems like the parser is doing something weird with IN there.
> >> I wonder why you don't get an array of text constants in the IN case.
> >
> > I poked into this and determined that it happens because transformAExprIn
> > identifies the array type to use without considering whether an
> additional
> > coercion will have to happen before the array elements can be compared to
> > the lefthand input.
> >
> > I tried to fix that in a principled fashion, by resolving the actual
> > comparison operator and using its righthand input type as the array
> > element type (see first patch attached).  That fixes this case all right,
> > but it also makes several existing regression test cases fail, for
> > example:
> >
> > ***
> > *** 381,392 
> >  FROM pg_class
> >  WHERE oid::regclass IN ('a_star', 'c_star')
> >  ORDER BY 1;
> > !  relname | has_toast_table
> > ! -+-
> > !  a_star  | t
> > !  c_star  | t
> > ! (2 rows)
> > !
> >   --UPDATE b_star*
> >   --   SET a = text 'gazpacho'
> >   --   WHERE aa > 4;
> > --- 381,389 
> >  FROM pg_class
> >  WHERE oid::regclass IN ('a_star', 'c_star')
> >  ORDER BY 1;
> > ! ERROR:  invalid input syntax for type oid: "a_star"
> > ! LINE 3:WHERE oid::regclass IN ('a_star', 'c_star')
> > !^
> >   --UPDATE b_star*
> >   --   SET a = text 'gazpacho'
> >   --   WHERE aa > 4;
> >
> > The problem is that regclass, like varchar, has no comparison operators
> > of its own, relying on OID's operators.  So this patch causes us to
> choose
> > OID not regclass as the type of the unknown literals, which in this case
> > seems like a loss of useful behavior.
>
> Agreed; no need to break that.
>
> > I'm tempted to just improve the situation for varchar with a complete
> > kluge, ie the second patch attached.  Thoughts?
>
> Fixes for me.
>
> Thanks,
> Amit
>

Re: [HACKERS] IF (NOT) EXISTS in psql-completion

[Robert Haas]

On Thu, Apr 7, 2016 at 8:19 AM, Kyotaro HORIGUCHI
 wrote:
> Thank you for looking this and for the comment.
>
> Since the end of this CF is quite soon and this seems in
> uncommittable state, feel free to move this to the next CF if any
> other patch with more priority.

Moved.  It's appropriate to consider this "needs review" at this
point, I think so added to 2016-09 that way.

-- 
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 v12] GSSAPI encryption support

[Robert Haas]

On Thu, Apr 7, 2016 at 10:17 PM, Michael Paquier
 wrote:
> On Thu, Apr 7, 2016 at 8:20 AM, Tom Lane  wrote:
>> Robbie Harwood  writes:
>>> Tom Lane  writes:
 Wait a second.  So the initial connection-request packet is necessarily
 unencrypted under this scheme?
>>
>>> Yes, by necessity.  The username must be sent in the clear, even if only
>>> as part of the GSSAPI handshake (i.e., the GSSAPI username will appear
>>> in plantext in the GSSAPI blobs which are otherwise encrypted).  GSSAPI
>>> performs authentication before it can start encryption.
>>
>> Ugh.  I had thought we were putting work into this because it represented
>> something we could recommend as best practice, but now you're telling me
>> that it's always going to be inferior to what we have already.
>
> It does not seem necessary to have an equivalent of
> pqsecure_open_client, just some extra handling in fe-connect.c to set
> up the initial context with a proper message handling... Not that
> direct anyway. So should the patch be marked as returned with feedback
> at this stage?

Yeah, I think so.  It doesn't seem we have consensus on this, and it's
too late to be trying to build one now.

-- 
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] raw output from copy

[Pavel Stehule]

2016-04-08 20:13 GMT+02:00 Robert Haas :

> On Tue, Apr 5, 2016 at 4:45 AM, Pavel Stehule 
> wrote:
> > here is cleaned/finished previous implementation of RAW_TEXT/RAW_BINARY
> > formats for COPY statements.
> >
> > The RAW with text formats means unescaped data, but with correct
> encoding -
> > input/output is realised with input/output function. RAW binary means
> > content produced/received by sending/received functions.
> >
> > Now both directions (input/output) working well
> >
> > Some examples of expected usage:
> >
> > copy (select xmlelement(name foo, 'hello')) to stdout (format raw_binary,
> > encoding 'latin2');
> >
> > create table avatars(id serial, picture bytea);
> > \copy avatars(picture) from ~/images/foo.jpg (format raw_binary);
> > select lastval();
> >
> > create table doc(id serial, txt text);
> > \copy doc(txt) from ~/files/aaa.txt (format raw_text, encoding 'latin2');
> > select lastval();
>
> As much as I know you and some other people would like it to be
> otherwise, this patch clearly does not have a sufficient degree of
> consensus to justify committing it to PostgreSQL 9.6.  I'm marking it
> Returned with Feedback.
>

ok, I'll try to complete this patch

Regards

Pavel


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

Re: [HACKERS] Fix for OpenSSL error queue bug

[Peter Geoghegan]

On Fri, Apr 8, 2016 at 11:12 AM, Peter Eisentraut  wrote:
> I have committed this so that the comments are only in the first instance in
> each file.  I think that should give enough information to someone who is
> curious about the details of the error handling.
>
> Also, I have adjusted this so that we check for n<0 after SSL_read and
> SSL_write, as you had in the frontend code but not in the backend code.

That seems reasonable. I'm glad we finally got this done. Thanks.


-- 
Peter Geoghegan


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

Re: [HACKERS] raw output from copy

[Robert Haas]

On Tue, Apr 5, 2016 at 4:45 AM, Pavel Stehule  wrote:
> here is cleaned/finished previous implementation of RAW_TEXT/RAW_BINARY
> formats for COPY statements.
>
> The RAW with text formats means unescaped data, but with correct encoding -
> input/output is realised with input/output function. RAW binary means
> content produced/received by sending/received functions.
>
> Now both directions (input/output) working well
>
> Some examples of expected usage:
>
> copy (select xmlelement(name foo, 'hello')) to stdout (format raw_binary,
> encoding 'latin2');
>
> create table avatars(id serial, picture bytea);
> \copy avatars(picture) from ~/images/foo.jpg (format raw_binary);
> select lastval();
>
> create table doc(id serial, txt text);
> \copy doc(txt) from ~/files/aaa.txt (format raw_text, encoding 'latin2');
> select lastval();

As much as I know you and some other people would like it to be
otherwise, this patch clearly does not have a sufficient degree of
consensus to justify committing it to PostgreSQL 9.6.  I'm marking it
Returned with Feedback.

-- 
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] Fix for OpenSSL error queue bug

[Peter Eisentraut]

On 04/07/2016 09:36 PM, Peter Geoghegan wrote:

On Thu, Apr 7, 2016 at 6:08 PM, Peter Eisentraut  wrote:

I wish we could avoid the huge, repeated comment blocks.  Perhaps we could
put them at the top of the files once?


I'm fine with that. Do you want to take care of that, or should I?


I have committed this so that the comments are only in the first 
instance in each file.  I think that should give enough information to 
someone who is curious about the details of the error handling.


Also, I have adjusted this so that we check for n<0 after SSL_read and 
SSL_write, as you had in the frontend code but not in the backend code.




--
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] Proposal: BSD Authentication support

[Tom Lane]

Marisa Emerson  writes:
> Woops, fix attached.

Pushed with minor adjustments.

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] Combining Aggregates

[Robert Haas]

On Fri, Apr 8, 2016 at 11:57 AM, Robert Haas  wrote:
> On Wed, Apr 6, 2016 at 5:28 PM, David Rowley
>  wrote:
>>> Is that everything now? I don't see anything about combining aggs in the git
>>> log and this is still showing as UnCommitted in the CF app.
>>
>> There's just a documentation patch and two combine functions for
>> floating point aggs left now (Haribabu's patch)
>
> I'll go have a look at that now.  Hopefully it will be in good shape
> and committable; if not, it'll have to wait for 9.7.

And... it's pretty hard to argue with any of this.  Committed.

I am *so* glad to be done with this patch series.  Man, that was a lot of work.

-- 
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] Choosing parallel_degree

[Robert Haas]

On Fri, Apr 8, 2016 at 12:27 PM, Paul Ramsey  wrote:
> Insofar as the patch is throttling how many parallel workers you get
> based solely on your relsize, it does concern this patch, but it's a
> general issue in both the extreme and not obviously related costings
> needed to trip parallel sequence and parallel aggregate plans. The
> parallel join seems to not take function/operator costs into account
> at all [3], at least I couldn't plump up a high enough cost to trip it
> without also adjusting the global parallel tuple cost configuration.
>
> I've seen a number of asides to the effect that "yes, costs are
> important, but we probably can't do anything about that for 9.6" in
> parallel patch threads, including this one, so I'm getting concerned
> that the core improvement we've been hoping for for years won't
> actually address our use cases when it is first released. That may
> just be the way it is, c'est la vie, but it would be unfortunate.

So, I don't think that the patch makes anything worse for PostGIS.  At
most, it doesn't make anything better for PostGIS.  Without the patch,
the number of workers in a parallel plan is based strictly on the size
of the relation upon which the parallel plan performs a parallel
sequential scan.  With the patch, you can - if you wish - substitute
some other number for the one the planner comes up with.  But you
don't have to do that, so it can't be worse than before.  To the
contrary, I'm fairly confident that it is useful, if not everything
you might ever have wanted.

Now, I think you really want is for the planner to be much smarter on
a broad level in determining when to use parallelism and how many
workers to pick on a per-query basis, and to take function and
operator costs into account in making that determination.  I agree
that's something that the planner should do, but it's definitely not
going to happen in 9.6.  It's going to require major new planner
infrastructure that will probably take years to build and get right,
unless Tom gets interested (he said hopefully).

There are basically two ways that a query can use too many workers.
First, at some point, when you add workers to a plan tree, the new and
larger number of workers don't produce tuples any faster in the
aggregate than a smaller number of workers would have done.  For
example, if a scan becomes I/O bound, adding more workers probably
won't help much.  Second, at some point, the workers you already have
are producing tuples as fast as the leader can consume them, and
producing them faster doesn't do you any good because the extra
workers will just spend time waiting for the leader to get around to
servicing them.

I have an idea about how to solve the first problem, which I've
started to mock up as a PoC.  What it does is track the portion of the
cost of each node that is believed to represent non-parallelizable
work, hereinafter serial_cost.  So if the query involves a lot of
CPU-intensive functions, then the ration of total_cost / serial_cost
will be high, suggesting that many workers will be useful.  If that
ratio is low, it suggests that the query is mostly I/O-bound or
IPC-bound anyway, and adding workers isn't going to do much.  Details
are still a bit fuzzy, and I don't have this developed enough that
it's actually testable in any way yet, but that's my idea.

The second problem is substantially harder, from what I can see.  To
judge how whether the leader can keep up, you need to know how much
CPU work it will have to do per gathered tuple.  In order to know
that, you'd need to know the costs of the plan nodes above the Gather
before knowing the cost of the Gather itself.  That's obviously a bit
challenging since our costing code doesn't work anything like that.
To some extent, this circularity is also an issue for the first
problem, since the costing of the lower nodes depending on how many
workers you have, but the number of workers you want at the point
where you Gather is dependent on the costing of those same lower
nodes.

I think there's probably a way through this mess - other people have
written good parallel optimizers before - but it is not like there is
some reasonably small tweak that will give you what you are hoping for
here.  Figuring out how all of this needs to work is a massive project
in its own right.

-- 
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] VS 2015 support in src/tools/msvc

[Christian Ullrich]

* Andrew Dunstan wrote:


On 04/08/2016 11:02 AM, Christian Ullrich wrote:



  src/port/chklocale.c(233): warning C4133: 'function': incompatible
  types - from 'const char *' to 'LPCWSTR' [...\postgres.vcxproj]



Do you have a fix for the LPCWSTR parameter issue?


As long as the locale short name cannot contain characters outside of 
ASCII, and I don't see how it could, just the typical 
measure-allocate-convert dance, add error handling to taste:


int res = MultiByteToWideChar(CP_ACP, 0, ctype, -1, NULL, 0);
WCHAR *wctype = malloc(res * sizeof(WCHAR));
memset(wctype, 0, res * sizeof(WCHAR));
res = MultiByteToWideChar(CP_ACP, 0, ctype, -1, wctype, wctypelen);

If it is somehow guaranteed that ctype is only the most basic short name 
("xx-YY") with no code pages or anything, it becomes much simpler, of 
course, and I would just use a loop.


If the locale name can contain characters above 0x7f, we'd have to know 
the code page of the string we use to get the code page.


--
Christian



--
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] Lower msvc build verbosity level

[Andrew Dunstan]

On 04/08/2016 12:24 PM, Christian Ullrich wrote:

* Tom Lane wrote:

+several.  Grepping for compiler warnings, for example, is really 
painful
right now on any MSVC critter.  I've resorted to grepping for 
"warning C",

which skips the noise messages, but I'm never sure if I'm missing
something.


You miss all diagnostics from other tools than the compiler, for one 
thing.


There is a simple solution to that, however. MSBuild and VCBuild 
repeat all warnings and errors produced during the build at the bottom 
of the log. I just checked on mastodon (VS 2005, the oldest), and it 
does that already.


This depends on the whole build being done using a single solution 
file that contains all the individual projects, but there is no reason 
to assume we will start building individual projects instead, I assume.




Yeah, what is more on the whole this is going to be far more beneficial, 
because stuff just gets lost badly in the noise. I have committed the 
change.


cheers

andrew


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

Re: [HACKERS] 2016-03 Commitfest

[Gavin Flower]

On 09/04/16 02:58, Magnus Hagander wrote:
On Fri, Apr 8, 2016 at 4:56 PM, Robert Haas > wrote:


On Fri, Apr 8, 2016 at 10:06 AM, David Steele mailto:da...@pgmasters.net>> wrote:
> On 4/8/16 10:00 AM, Tom Lane wrote:
>> David Steele mailto:da...@pgmasters.net>>
writes:
>>> So the commitfest is 84% complete with less than twelve hours
to go.
>>
>> Have we set a particular time-of-day for closing the CF, and if so
>> what is it exactly?
>
> From the referenced email:
>
> "Accordingly, the release management has decided that all
> feature patches destined for PostgreSQL 9.6 must be committed no
later
> than April 8, 2016.  Any patch not committed prior to 2016-04-09
> 00:00:00 GMT may not be committed to PostgreSQL 9.6 unless (a)
it is a
> bug fix, (b) it represents essential cleanup of a
previously-committed
> patch, or (c) the release management team has approved an
extension to
> the deadline for that particular patch."

IOW, the deadline is 8pm US/Eastern time, or about 9 hours from now.

Let's try not to introduce more bugs in the next 9 hours than we have
in the preceding 9 months.


What? Where's the fun in that?!

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/
Yes, even worse Robert might attract the ire of the BLL (Bug Liberation 
League) - and then where would we be???





--
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] Move PinBuffer and UnpinBuffer to atomics

[Andres Freund]

On 2016-04-07 16:50:44 +0300, Alexander Korotkov wrote:
> On Thu, Apr 7, 2016 at 4:41 PM, Andres Freund  wrote:
>
> > On 2016-03-31 20:21:02 +0300, Alexander Korotkov wrote:
> > > ! BEGIN_BUFSTATE_CAS_LOOP(bufHdr);
> > >
> > > ! Assert(BUF_STATE_GET_REFCOUNT(state) > 0);
> > > ! wasDirty = (state & BM_DIRTY) ? true : false;
> > > ! state |= BM_DIRTY | BM_JUST_DIRTIED;
> > > ! if (state == oldstate)
> > > ! break;
> >
> > I'm doubtful that this early exit is entirely safe. None of the
> > preceding operations imply a memory barrier. The buffer could previously
> > have been marked dirty, but cleaned since. It's pretty critical that we
> > re-set the dirty bit (there's no danger of loosing it with a barrier,
> > because we hold an exclusive content lock).
> >
>
> Oh, I get it.
>
>
> > Practically the risk seems fairly low, because acquiring the exclusive
> > content lock will have implied a barrier. But it seems unlikely to have
> > a measurable performance effect to me, so I'd rather not add the early
> > exit.
> >
>
> Ok, let's just remove it.

Here's my updated version of the patch. I've updated this on an
intercontinental flight, after a otherwise hectic week (moving from SF
to Berlin); so I'm planning to look over this once more before pushing (.

I've decided that the cas-loop macros are too obfuscating for my
taste. To avoid duplicating the wait part I've introduced
WaitBufHdrUnlocked().

As you can see in
http://archives.postgresql.org/message-id/CA%2BTgmoaeRbN%3DZ4oWENLvgGLeHEvGZ_S_Z3KGrdScyKiSvNt3oA%40mail.gmail.com
I'm planning to apply this sometime this weekend, after running some
tests and going over the patch again.

Any chance you could have a look over this?

Regards,

Andres
>From e89e99acc5b0854f918f0c7f685efcd50e6ffcae Mon Sep 17 00:00:00 2001
From: Andres Freund 
Date: Thu, 7 Apr 2016 10:29:41 +0200
Subject: [PATCH] Allow Pin/UnpinBuffer to operate in a lockfree manner.

Pinning/Unpinning a buffer is a very frequent operation; especially in
read-mostly cache resident workloads. Benchmarking shows that in various
scenarios the spinlock protecting a buffer header's state becomes a
significant bottleneck. The problem can be reproduced with pgbench -S on
larger machines, but can be considerably worse for queries which touch
the same buffers over and over at a high frequency (e.g. nested loops
over a small inner table).

To allow atomic operations to be used, cram BufferDesc's flags,
usage_count, buf_hdr_lock, refcount into a single 32bit atomic variable;
that allows to manipulate them together using 32bit compare-and-swap
operations. This requires reducing MAX_BACKENDS to 2^18-1 (which could
be lifted by using a 64bit field, but it's not a realistic configuration
atm).

As not all operations can easily implemented in a lockfree manner,
implement the previous buf_hdr_lock via a flag bit in the atomic
variable. That way we can continue to lock the header in places where
it's needed, but can get away without acquiring it in the more frequent
hot-paths.  There's some additional operations which can be done without
the lock, but aren't in this patch; but the most important places are
covered.

As bufmgr.c now essentially re-implements spinlocks, abstract the delay
logic from s_lock.c into something more generic. It now has already two
users, and it seems likely that more are coming up; there's pending
patches for lwlock.c at least.

This patch is based on a proof-of-concept written by me, which Alexander
Korotkov made into a fully working patch; the committed version is again
revised by me.  Benchmarking and testing has, amongst others, been
provided by Dilip Kumar, Alexander Korotkov, Robert Haas.

Author: Alexander Korotkov and Andres Freund
Discussion: 2400449.GjM57CE0Yg@dinodell
---
 contrib/pg_buffercache/pg_buffercache_pages.c |  15 +-
 src/backend/storage/buffer/buf_init.c |  18 +-
 src/backend/storage/buffer/bufmgr.c   | 491 +-
 src/backend/storage/buffer/freelist.c |  44 ++-
 src/backend/storage/buffer/localbuf.c |  64 ++--
 src/backend/storage/lmgr/s_lock.c | 218 ++--
 src/include/postmaster/postmaster.h   |  15 +-
 src/include/storage/buf_internals.h   | 101 --
 src/include/storage/s_lock.h  |  18 +
 src/tools/pgindent/typedefs.list  |   1 +
 10 files changed, 621 insertions(+), 364 deletions(-)

diff --git a/contrib/pg_buffercache/pg_buffercache_pages.c b/contrib/pg_buffercache/pg_buffercache_pages.c
index 6622d22..17b4b6f 100644
--- a/contrib/pg_buffercache/pg_buffercache_pages.c
+++ b/contrib/pg_buffercache/pg_buffercache_pages.c
@@ -148,11 +148,12 @@ pg_buffercache_pages(PG_FUNCTION_ARGS)
 		 */
 		for (i = 0; i < NBuffers; i++)
 		{
-			volatile BufferDesc *bufHdr;
+			BufferDesc *bufHdr;
+			uint32		buf_state;
 
 			bufHdr = GetBufferDescriptor(i);
 			/* Lock each buffer header before inspecting. */
-			LockBufHdr(bufHdr);
+	

Re: [HACKERS] Proposal: BSD Authentication support

[Tom Lane]

David Steele  writes:
> On 4/8/16 11:20 AM, Robert Haas wrote:
>> Should we commit this patch?

> To summarize:

> Robert Haas and Peter Eisentraut have done code-only reviews.  Thomas
> Munro has reviewed and tested with a caveat that he is no BSD expert.
> Pierre-Emmanuel tested but did not do a code review review due to his
> unfamiliarity with the Postgres code-base.

> The patch does not seem invasive or likely to cause problems on non-BSD
> systems.  From my perspective this has gotten about as much review as it
> can, so +1 from me at least.

Given our lack of BSD developers, there's probably not much more we can
do, but one more set of eyeballs on the patch wouldn't hurt.  I'll take
it, unless somebody else wants to.

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] Choosing parallel_degree

[Paul Ramsey]

On Fri, Apr 8, 2016 at 9:06 AM, Simon Riggs  wrote:
> On 8 April 2016 at 17:00, Paul Ramsey  wrote:
>>
>> On Fri, Apr 8, 2016 at 8:23 AM, Robert Haas  wrote:
>> > On Fri, Apr 8, 2016 at 1:22 AM, Amit Kapila 
>> > wrote:
>> >> Other than that, patch looks good and I have marked it as Ready For
>> >> Committer.  Hope, we get this for 9.6.
>> >
>> > Committed.  I think this is likely to make parallel query
>> > significantly more usable in 9.6.
>>
>> I'm kind of worried that it will make it yet less usable for PostGIS,
>> since approaches that ignore costs in favour of relpages will
>> dramatically under-resource our queries. I can spin a query for
>> multiple seconds on a table with less than 100K records, not even
>> trying very hard.
>
> Doesn't sound good.

I admit, it's not a "usual" database thing, but it's right in the
meaty middle of use cases that parallelism can crushingly awesomely
defeat. It's also probably not too unusual for extension use cases,
where complex data are held in user defined types, whether they be
image fragments, music samples, genetic data, raster data or LIDAR
point clouds. PostGIS is just one voice of many in the Symphony of
Crazy Shit in the Database.

>> Functions have very unequal CPU costs, and we're talking here about
>> using CPUs more effectively, why are costs being given the see-no-evil
>> treatment? This is as true in core as it is in PostGIS, even if our
>> case is a couple orders of magnitude more extreme: a filter based on a
>> complex combination of regex queries will use an order of magnitude
>> more CPU than one that does a little math, why plan and execute them
>> like they are the same?
>
> Functions have user assignable costs.

We have done a relatively bad job of globally costing our functions
thus far, because it mostly didn't make any difference. In my testing
[1], I found that costing could push better plans for parallel
sequence scans and parallel aggregates, though at very extreme cost
values (1000 for sequence scans and 1 for aggregates)

Obviously, if costs can make a difference for 9.6 and parallelism
we'll rigorously ensure we have good, useful costs. I've already
costed many functions in my parallel postgis test branch [2]. Perhaps
the avoidance of cost so far is based on the relatively nebulous
definition it has: about the only thing in the docs is "If the cost is
not specified, 1 unit is assumed for C-language and internal
functions, and 100 units for functions in all other languages. Larger
values cause the planner to try to avoid evaluating the function more
often than necessary."

So what about C functions then? Should a string comparison be 5 and a
multiplication 1? An image histogram 1000?

>> As it stands now, it seems like out of the box PostGIS users will
>> actually not see much benefit from parallelism unless they  manhandle
>> their configuration settings to force it.
>
> Does this concern apply to this patch, or to the general situation for 9.6.

Insofar as the patch is throttling how many parallel workers you get
based solely on your relsize, it does concern this patch, but it's a
general issue in both the extreme and not obviously related costings
needed to trip parallel sequence and parallel aggregate plans. The
parallel join seems to not take function/operator costs into account
at all [3], at least I couldn't plump up a high enough cost to trip it
without also adjusting the global parallel tuple cost configuration.

I've seen a number of asides to the effect that "yes, costs are
important, but we probably can't do anything about that for 9.6" in
parallel patch threads, including this one, so I'm getting concerned
that the core improvement we've been hoping for for years won't
actually address our use cases when it is first released. That may
just be the way it is, c'est la vie, but it would be unfortunate.

P

[1] http://blog.cleverelephant.ca/2016/03/parallel-postgis.html
[2] https://github.com/pramsey/postgis/tree/parallel
[3] http://blog.cleverelephant.ca/2016/03/parallel-postgis-joins.html


-- 
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] Speed up Clog Access by increasing CLOG buffers

[Amit Kapila]

On Fri, Apr 8, 2016 at 9:00 PM, Andres Freund  wrote:
>
> On 2016-03-31 15:07:22 +0530, Amit Kapila wrote:
> > I think we should change comments on top of this function.  I have
changed
> > the comments as per my previous patch and attached the modified patch
with
> > this mail, see if that makes sense.
>
> I've applied this patch.
>

Thanks!


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

Re: [HACKERS] Lower msvc build verbosity level

[Christian Ullrich]

* Tom Lane wrote:


+several.  Grepping for compiler warnings, for example, is really painful
right now on any MSVC critter.  I've resorted to grepping for "warning C",
which skips the noise messages, but I'm never sure if I'm missing
something.


You miss all diagnostics from other tools than the compiler, for one thing.

There is a simple solution to that, however. MSBuild and VCBuild repeat 
all warnings and errors produced during the build at the bottom of the 
log. I just checked on mastodon (VS 2005, the oldest), and it does that 
already.


This depends on the whole build being done using a single solution file 
that contains all the individual projects, but there is no reason to 
assume we will start building individual projects instead, I assume.


--
Christian



--
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] Batch update of indexes

[David Steele]

On 4/2/16 4:21 PM, Konstantin Knizhnik wrote:

> Thank you for review, Tom.
> 
> I completely agree with all your arguments against this patch.
> I have proposed this patch mostly as prove of concept.

I have marked this "returned with feedback".  Hopefully you can work on
the concept and resubmit for 9.7.

Thanks,
-- 
-David
da...@pgmasters.net


-- 
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] VS 2015 support in src/tools/msvc

[Andrew Dunstan]

On 04/08/2016 11:02 AM, Christian Ullrich wrote:

* Michael Paquier wrote:

On Fri, Apr 8, 2016 at 10:05 PM, Andrew Dunstan  
wrote:

¥> On 04/08/2016 07:15 AM, Christian Ullrich wrote:


Michael, none of your patches change this, so how does it ever 
build on

your system?


Luck. I am getting a warning but the code is able to somewhat compile:
   src/port/chklocale.c(230): warning C4013: 'GetLocaleInfoEx'
undefined; assuming extern returning int
[C:\Users\IEUser\git\postgres\libpgport.vcxproj]


Weird. This assumed declaration is __cdecl, the actual function is 
__stdcall, and I think this should be guaranteed to crash.



But that's clearly incorrect to get that. As you are saying, what we
actually just need to do is bumping _WIN32_WINNT to 0x0600 when
compiling with VS2015, meaning that the minimum build requirement for
Postgres with VS2015 would be Windows Vista, and it would not be
possible to compile it on XP or Windows server 2k3. As XP is already
out of support, I think that this is an acceptable tradeoff, and it
would still be possible to build Postgres on XP with older versions of
Visual. Thoughts?


I think you confuse two things here, let's call them "build 
environment" and "build platform". The build environment is whichever 
Windows SDK (among other things) is installed; if it is a version for 
Vista or later, that just means it has the declaration in the first 
place, and has the import in kernel32.lib. The build platform is the 
OS the compiler is run on; as long as you find a compiler that 
understands the headers from your chosen SDK version, you can run it 
on Windows 95 if both of you want.


Changing _WIN32_WINNT also affects, indirectly, on which platforms the 
resulting binaries can run. Assume a macro that has an alternative 
definition, conditioned on _WIN32_WINNT >= _WIN32_WINNT_VISTA, that 
uses a function added in Vista. A binary built using this declaration, 
no matter where and when, will not run on anything older.



Anyway, attached are updated patches. This makes the warning go away
from my side, so I guess that it should allow Andrew to compile the
code.


Which brings us to the next problem:

  src/port/chklocale.c(233): warning C4133: 'function': incompatible
  types - from 'const char *' to 'LPCWSTR' [...\postgres.vcxproj]

I have no idea why I get this warning; I would have expected something 
more like this:


  localetest.cpp(26): error C2664: 'int
  GetLocaleInfoEx(LPCWSTR,LCTYPE,LPWSTR,int)': cannot convert
  argument 1 from 'const char *' to 'LPCWSTR'

Apparently the warning is triggered by type mismatches in pointer 
arithmetic, although I can't see any here. Anyway, it concerns the 
first argument in this call to GetLocaleInfoEx(), which here is a 
const char*.


According to the documentation and the prototype, however, it should 
be an LPCWSTR, because this function is Unicode-only (has no A/W 
variants). Unless LOCALE_IDEFAULTANSICODEPAGE also changes the 
interpretation of this first argument to a single-byte string, which 
is not mentioned anywhere in the documentation and makes no sense to 
begin with, I don't think this has ever worked either.


I just tested it, and, of course, if I pass '(LPCWSTR)"de-DE"' (narrow 
string cast to LPCWSTR), the call fails with ERROR_INVALID_PARAMETER. 
With a wide string, I get the correct code page for the locale.



Also, while you're at it, could you improve the comments a bit? I have 
not yet tried following the code to see which locale formats it uses 
where ("German_Germany", "de-DE", etc.), but GetLocaleInfoEx() takes 
the short form and there is a comment about the long form right below 
that call once patched (in the old code that gets turned into an else 
branch).






OK, well, we're making progress. I can confirm that using _WIN32_WINNT = 
0x0600 fixes my problems - I can build and run the regression tests. I'm 
inclined to define _WINSOCK_DEPRECATED_NO_WARNINGS to silence a few 
compiler bleatings.


Do you have a fix for the LPCWSTR parameter issue?

cheers

andrew


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

Re: [HACKERS] Choosing parallel_degree

[Simon Riggs]

On 8 April 2016 at 17:00, Paul Ramsey  wrote:

> On Fri, Apr 8, 2016 at 8:23 AM, Robert Haas  wrote:
> > On Fri, Apr 8, 2016 at 1:22 AM, Amit Kapila 
> wrote:
> >> Other than that, patch looks good and I have marked it as Ready For
> >> Committer.  Hope, we get this for 9.6.
> >
> > Committed.  I think this is likely to make parallel query
> > significantly more usable in 9.6.
>
> I'm kind of worried that it will make it yet less usable for PostGIS,
> since approaches that ignore costs in favour of relpages will
> dramatically under-resource our queries. I can spin a query for
> multiple seconds on a table with less than 100K records, not even
> trying very hard.
>

Doesn't sound good.


> Functions have very unequal CPU costs, and we're talking here about
> using CPUs more effectively, why are costs being given the see-no-evil
> treatment? This is as true in core as it is in PostGIS, even if our
> case is a couple orders of magnitude more extreme: a filter based on a
> complex combination of regex queries will use an order of magnitude
> more CPU than one that does a little math, why plan and execute them
> like they are the same?
>

Functions have user assignable costs.


> As it stands now, it seems like out of the box PostGIS users will
> actually not see much benefit from parallelism unless they  manhandle
> their configuration settings to force it.
>

Does this concern apply to this patch, or to the general situation for 9.6.

Please suggest what you would like to see.

-- 
Simon Riggshttp://www.2ndQuadrant.com/

PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: [HACKERS] Choosing parallel_degree

[Paul Ramsey]

On Fri, Apr 8, 2016 at 8:23 AM, Robert Haas  wrote:
> On Fri, Apr 8, 2016 at 1:22 AM, Amit Kapila  wrote:
>> Other than that, patch looks good and I have marked it as Ready For
>> Committer.  Hope, we get this for 9.6.
>
> Committed.  I think this is likely to make parallel query
> significantly more usable in 9.6.

I'm kind of worried that it will make it yet less usable for PostGIS,
since approaches that ignore costs in favour of relpages will
dramatically under-resource our queries. I can spin a query for
multiple seconds on a table with less than 100K records, not even
trying very hard.

Functions have very unequal CPU costs, and we're talking here about
using CPUs more effectively, why are costs being given the see-no-evil
treatment? This is as true in core as it is in PostGIS, even if our
case is a couple orders of magnitude more extreme: a filter based on a
complex combination of regex queries will use an order of magnitude
more CPU than one that does a little math, why plan and execute them
like they are the same?

As it stands now, it seems like out of the box PostGIS users will
actually not see much benefit from parallelism unless they  manhandle
their configuration settings to force it.

ATB,

P


-- 
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] 2016-03 Commitfest

[Simon Riggs]

On 8 April 2016 at 16:00, Andres Freund  wrote:

I've finished polishing the Pin/Unpin patch. But the final polishing
> happened on an intercontential flight, after days spent preparing my
> move to SF. I'd be glad if you would allow me to look over the patch
> again, before pushing it sometime this weekend; this stuff is subtle,
> and I'm not exactly my best right now.
>

I think you should push it now then.

Two reasons

1) We shouldn't be violating the deadline the first time its been in place.
It just creates a precedent that sometimes the deadlines don't matter and
that's not a useful position.

2) If you commit what you have, someone else might be able to see a bug you
cannot

-- 
Simon Riggshttp://www.2ndQuadrant.com/

PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: [HACKERS] Combining Aggregates

[Robert Haas]

On Wed, Apr 6, 2016 at 5:28 PM, David Rowley
 wrote:
>> Is that everything now? I don't see anything about combining aggs in the git
>> log and this is still showing as UnCommitted in the CF app.
>
> There's just a documentation patch and two combine functions for
> floating point aggs left now (Haribabu's patch)

I'll go have a look at that now.  Hopefully it will be in good shape
and committable; if not, it'll have to wait for 9.7.

-- 
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] multivariate statistics v14

[Robert Haas]

On Tue, Mar 29, 2016 at 11:18 AM, David Steele  wrote:
> On 3/28/16 4:42 AM, Tomas Vondra wrote:
>> Yes, those are valid omissions. I plan to address them, and I'd also
>> considering adding a section to 65.1 (How the Planner Uses Statistics),
>> explaining more thoroughly how the planner uses multivariate stats.
>
> It looks you need post a new patch so I have marked this "waiting on
> author".

Since no new version of this patch has been posted in the last 10
days, it seems clear that there will not be time for this to
reasonably become ready for committer and then get committed in the
few hours remaining before the deadline.  That is a bummer, since I
was hoping we would have this feature in this release, but hopefully
we will get it into 9.7.  I am marking it Returned with Feedback.

-- 
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] Proposal: BSD Authentication support

[David Steele]

On 4/8/16 11:20 AM, Robert Haas wrote:
> On Fri, Apr 8, 2016 at 6:48 AM, Pierre-Emmanuel André  
> wrote:
>> On Fri, Mar 18, 2016 at 06:30:35PM +1300, Thomas Munro wrote:
>>> On Fri, Mar 18, 2016 at 12:49 PM, Marisa Emerson  wrote:
 On 18/03/16 03:57, Thomas Munro wrote:
>
> You used one name in the docs and another in the code:
>
> +BSD Authentication on PostgreSQL uses the
> auth-postgres
> +login type and authenticates with the postgres
> login
>
> + retval = auth_userokay(user, NULL, "auth-postgresql", passwd);


 Woops, fix attached.
>>>
>>> Thanks!
>>>
>>> I'm CCng Pierre-Emmanuel André, maintainer of the OpenBSD postgresql
>>> port/package, just in case he has any feedback.
>>>
>>> Pierre-Emmanuel, here's the full thread in case you missed it:
>>> http://www.postgresql.org/message-id/1521c2f9465.e357a543197915.6912077634413325...@insec.sh
>>
>> Sorry for the late answer.
>> I've tested the patch on @amd64 with the latest PostgreSQL 9.6devel. I can't 
>> judge the diff
>> itself (it looks good but i'm not a PostgreSQL developper) but everything 
>> works fine.
>> It would be a nice feature to have on OpenBSD.
> 
> Opinion poll:
> 
> Should we commit this patch?

To summarize:

Robert Haas and Peter Eisentraut have done code-only reviews.  Thomas
Munro has reviewed and tested with a caveat that he is no BSD expert.
Pierre-Emmanuel tested but did not do a code review review due to his
unfamiliarity with the Postgres code-base.

The patch does not seem invasive or likely to cause problems on non-BSD
systems.  From my perspective this has gotten about as much review as it
can, so +1 from me at least.

-- 
-David
da...@pgmasters.net



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] 2016-03 Commitfest

[Robert Haas]

On Fri, Apr 8, 2016 at 11:00 AM, Andres Freund  wrote:
> On 2016-04-08 10:56:28 -0400, Robert Haas wrote:
>> On Fri, Apr 8, 2016 at 10:06 AM, David Steele  wrote:
>> > On 4/8/16 10:00 AM, Tom Lane wrote:
>> >> David Steele  writes:
>> >>> So the commitfest is 84% complete with less than twelve hours to go.
>> >>
>> >> Have we set a particular time-of-day for closing the CF, and if so
>> >> what is it exactly?
>> >
>> > From the referenced email:
>> >
>> > "Accordingly, the release management has decided that all
>> > feature patches destined for PostgreSQL 9.6 must be committed no later
>> > than April 8, 2016.  Any patch not committed prior to 2016-04-09
>> > 00:00:00 GMT may not be committed to PostgreSQL 9.6 unless (a) it is a
>> > bug fix, (b) it represents essential cleanup of a previously-committed
>> > patch, or (c) the release management team has approved an extension to
>> > the deadline for that particular patch."
>>
>> IOW, the deadline is 8pm US/Eastern time, or about 9 hours from now.
>>
>> Let's try not to introduce more bugs in the next 9 hours than we have
>> in the preceding 9 months.
>
> I've finished polishing the Pin/Unpin patch. But the final polishing
> happened on an intercontential flight, after days spent preparing my
> move to SF. I'd be glad if you would allow me to look over the patch
> again, before pushing it sometime this weekend; this stuff is subtle,
> and I'm not exactly my best right now.

In view of these circumstances, the RMT has voted to extend the
deadline for this particular patch by 2.5 days; that is, this patch
may be committed with RMT approval no later than 2016-04-11 12:00:00
GMT, which I believe is approximately 4am Monday morning where you
are.

Robert Haas
PostgreSQL 9.6 Release Management Team


-- 
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] speeding up GIN build with parallel workers

[Robert Haas]

On Fri, Apr 8, 2016 at 10:04 AM, Constantin S. Pan  wrote:
> Here is a new version of the patch, which:
>
> 1. Fixes some minor stylistic issues.
>
> 2. Uses binaryheap (instead of a custom ugly stack) for merging.

I think we need to push this patch out to 9.7.  This code has had a
little review here and there from various people, but clearly not
enough to push it into the tree at the very last minute.  I don't
think we even have enough discussion to conclude that things like the
gin_shared_mem GUCs are good ideas rather than bad ones.

Also, I personally find this code to be extremely low on comments.
There's barely any explanation of what the overall methodology is
here.  Most of the functions do not have a header comment explaining
what they do.  The code hasn't been run through pgindent.  There's no
check to see whether the computation that will be done inside the GIN
index is parallel-safe; what if it's an expression index on an unsafe
function?  Opening the heap and index with no lock in the worker is
wrong; the worker should use the same lock mode as the leader.

That's just on a quick read-through; I'm sure there's more.  I'm going
to move this to the next CommitFest.  Hopefully someone will volunteer
to do some serious review of this, because the feature sounds cool.
Possibly that person will even be me.  But it will not be today.

-- 
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] proposal: PL/Pythonu - function ereport

[Teodor Sigaev]

Thank you very much


thank you, pushed. Pls, pay attention to buildfarm.

--
Teodor Sigaev   E-mail: teo...@sigaev.ru
   WWW: http://www.sigaev.ru/


--
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] Speed up Clog Access by increasing CLOG buffers

[Andres Freund]

On 2016-04-08 13:07:05 +0530, Amit Kapila wrote:
> I think by now, we have done many tests with both approaches and we find
> that in some cases, it is slightly better and in most cases it is neutral
> and in some cases it is worse than group clog approach.  I feel we should
> go with group clog approach now as that has been tested and reviewed
> multiple times and in future if we find that other approach is giving
> substantial gain, then we can anyway change it.

I think that's a discussion for the 9.7 cycle unfortunately. I've now
pushed the #clog-buffers patch; that's going to help the worst cases.

Greetings,

Andres Freund


-- 
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] Speed up Clog Access by increasing CLOG buffers

[Andres Freund]

On 2016-03-31 15:07:22 +0530, Amit Kapila wrote:
> I think we should change comments on top of this function.  I have changed
> the comments as per my previous patch and attached the modified patch with
> this mail, see if that makes sense.

I've applied this patch.

Regards,

Andres


-- 
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] Execute ignoring cursor?

[Pavel Stehule]

2016-04-08 16:46 GMT+02:00 nummervet nummervet :

> That didn't work for me:
>
> ERROR:  syntax error at or near "$"
> LINE 1: ...ibute_id, set_id ) (select $."151", '...
>

should be $1

Regards

Pavel


>
>
> Пятница, 8 апреля 2016, 17:25 +03:00 от Pavel Stehule <
> pavel.steh...@gmail.com>:
>
>
> Hi
>
> 2016-04-08 16:17 GMT+02:00 nummervet nummervet  >:
>
> Hello. Didn't find dedicated plpgsql list, so decided to post question
> here.
> I am trying to create a  function that will pick up some values from
> cursor and execute them as a dynamic query.
> However, once i use EXECUTE, its seems to be ignoring the existence of
> cursor and try to pick up values from table.
> Basically:
>
> insert into mytable ( value, attribute_id, set_id ) (select rec."151",
> '201', '1')
>
> works, but
>
> execute 'insert into mytable ( value, attribute_id, set_id ) (select
> rec."151", ''201'', ''1'')'
>
>
> Dynamic queries are executed in own space and there are not direct access
> to plpgsql variables.
>
> please, try: execute 'insert into mytable ( value, attribute_id, set_id )
> (select $1."151", ''201'', ''1'')' using rec;
>
> The content should be passed to dynamic query via USING clause.
>
>
> http://www.postgresql.org/docs/current/static/plpgsql-statements.html#PLPGSQL-STATEMENTS-EXECUTING-DYN
>
> Regards
>
> Pavel Stehule
>
>
>
> fails with
>
> ERROR:  missing FROM-clause entry for table "rec"
> LINE 1: ...ibute_id, set_id ) (select rec."151",...
>
> Is there any way around it? Or should i just give up and do it some other
> way?
>
>
>
>

Re: [HACKERS] Choosing parallel_degree

[Robert Haas]

On Fri, Apr 8, 2016 at 1:22 AM, Amit Kapila  wrote:
> Other than that, patch looks good and I have marked it as Ready For
> Committer.  Hope, we get this for 9.6.

Committed.  I think this is likely to make parallel query
significantly more usable in 9.6.

-- 
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] Proposal: BSD Authentication support

[Robert Haas]

On Fri, Apr 8, 2016 at 6:48 AM, Pierre-Emmanuel André  wrote:
> On Fri, Mar 18, 2016 at 06:30:35PM +1300, Thomas Munro wrote:
>> On Fri, Mar 18, 2016 at 12:49 PM, Marisa Emerson  wrote:
>> > On 18/03/16 03:57, Thomas Munro wrote:
>> >>
>> >> You used one name in the docs and another in the code:
>> >>
>> >> +BSD Authentication on PostgreSQL uses the
>> >> auth-postgres
>> >> +login type and authenticates with the postgres
>> >> login
>> >>
>> >> + retval = auth_userokay(user, NULL, "auth-postgresql", passwd);
>> >
>> >
>> > Woops, fix attached.
>>
>> Thanks!
>>
>> I'm CCng Pierre-Emmanuel André, maintainer of the OpenBSD postgresql
>> port/package, just in case he has any feedback.
>>
>> Pierre-Emmanuel, here's the full thread in case you missed it:
>> http://www.postgresql.org/message-id/1521c2f9465.e357a543197915.6912077634413325...@insec.sh
>
> Sorry for the late answer.
> I've tested the patch on @amd64 with the latest PostgreSQL 9.6devel. I can't 
> judge the diff
> itself (it looks good but i'm not a PostgreSQL developper) but everything 
> works fine.
> It would be a nice feature to have on OpenBSD.

Opinion poll:

Should we commit this patch?

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


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

Re: [HACKERS] WIP: Covering + unique indexes.

[Anastasia Lubennikova]

08.04.2016 15:45, Anastasia Lubennikova:

08.04.2016 15:06, Teodor Sigaev:

On Wed, Apr 6, 2016 at 1:50 PM, Peter Geoghegan  wrote:

Personally, I like documenting assertions, and will sometimes write
assertions that the compiler could easily optimize away. Maybe going
*that* far is more a matter of personal style, but I think an
assertion about the new index tuple size being <= the old one is just
a good idea. It's not about a problem in your code at all.


You should make index_truncate_tuple()/index_reform_tuple() promise to
always do this in its comments/contract with caller as part of this,
IMV.


Some notices:
- index_truncate_tuple(Relation idxrel, IndexTuple olditup, int 
indnatts,

   int  indnkeyatts)
  Why we need indnatts/indnkeyatts? They are presented in idxrel struct
  already
- follow code where index_truncate_tuple() is called, it should never 
called in
  case where indnatts == indnkeyatts. So, indnkeyatts should be 
strictly less
  than indnatts, pls, change assertion. If they are equal the this 
function

  becomes complicated variant of CopyIndexTuple()


Good point. These attributes seem to be there since previous versions 
of the function.

But now they are definitely unnecessary. Updated patch is attached


One more improvement - note about expressions into documentation.

--
Anastasia Lubennikova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

diff --git a/doc/src/sgml/ref/create_index.sgml b/doc/src/sgml/ref/create_index.sgml
index b5f67af..61a21a9 100644
--- a/doc/src/sgml/ref/create_index.sgml
+++ b/doc/src/sgml/ref/create_index.sgml
@@ -161,6 +161,8 @@ CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ [ IF NOT EXISTS ] INCLUDING clause, which can slightly reduce the size of the index,
 due to storing included attributes only in leaf index pages.
 Currently, only the B-tree access method supports this feature.
+Expressions as included columns are not supported since they cannot be used
+in index-only scan.

   
  

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