Re: [HACKERS] WIP: About CMake v2

[Euler Taveira]

On 26-11-2015 14:06, YUriy Zhuravlev wrote:
> On Thursday 26 November 2015 17:42:16 you wrote:
>> No point in doing any work if you don't agree with the basic prerequisites.
> I meant that support for older versions of CMake I'll do when will implement 
> other functions.
> 
I think you don't understand the point: start with the *right* cmake
version because you could have to redo (a lot of) your work or have your
patch rejected because you don't follow our advice.


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


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

Re: [HACKERS] New email address

[Tom Lane]

Stefan Kaltenbrunner  writes:
> that seems entirely doable with our current infrastructure (and even
> with minimal-to-no hackery on mj2) - but it still carries the "changes
> From:" issue :/

Yeah.  What do you think of the other approach of trying to preserve
validity of the incoming DKIM-Signature (in particular, by not changing
the Subject or message body)?

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] New email address

[Greg Stark]

On Wed, Nov 25, 2015 at 6:55 PM, Tom Lane  wrote:
> > But my point was that while the RFC says what to put there there's
> > absolutely no reference anywhere for when the information should cause
> > any MUA or MTA to behave differently.
>
> Agreed.  To my mind that's a reason why Sender should not be DKIM-signed.
> Unfortunately, RFC 6376 explicitly suggests doing so ... and it looks like
> some people are taking that advice.

Hm, I see it as a reason why signing Sender is reasonable. If it were
a functional header then there might be a reason it would have to be
changed. But if it's purely informational and the receiving MUA is
going to display to the user (which is a bad idea imho but Gmail and
Exchange both do it) then it makes sense to expect some authentication
for it. I think the thinking is basically "sign everything we're going
to present to the user phishers can't claim to be someone they're
not". In which case it's fairly important that things like Sender be
signed. Or that everyone agree it's just a useless header and stop
sending or displaying it.

I don't think we should base any action on guesses of what Gmail does.
Google may do something we don't expect that's more complex to work
around the problem. For one thing you can have email addresses at
Google from a number of domains so they may well be able to have more
than one policy for different users.

I would suggest we stop doing things that are obviously incompatible
with DKIM -- header and body munging for example. And I suspect we can
stop touching Sender without any ill effects too.

One idea might be to add a script to check a user's domain for
p=reject and send them a warning when subscribing (or sending mail to
the list?) warning them of the problem.

-- 
greg


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

[HACKERS] Errors in our encoding conversion tables

[Tom Lane]

There's a discussion over at
http://www.postgresql.org/message-id/flat/2sa.dhu5.1hk1yrptnfy.1ml...@seznam.cz
of an apparent error in our WIN1250 -> LATIN2 conversion.  I looked into this
and found that indeed, the code will happily translate certain characters
for which there seems to be no justification.  I made up a quick script
that would recompute the conversion tables in latin2_and_win1250.c from
the Unicode mapping files in src/backend/utils/mb/Unicode, and what it
computes is shown in the attached diff.  (Zeroes in the tables indicate
codes with no translation, for which an error should be thrown.)

Having done that, I thought it would be a good idea to see if we had any
other conversion tables that weren't directly based on the Unicode data.
The only ones I could find were in cyrillic_and_mic.c, and those seem to
be absolutely filled with errors, to the point where I wonder if they were
made from the claimed encodings or some other ones.  The attached patch
recomputes those from the Unicode data, too.

None of this data seems to have been touched since Tatsuo-san's original
commit 969e0246, so it looks like we simply didn't vet that submission
closely enough.

I have not attempted to reverify the files in utils/mb/Unicode against the
original Unicode Consortium data, but maybe we ought to do that before
taking any further steps here.

Anyway, what are we going to do about this?  I'm concerned that simply
shoving in corrections may cause problems for users.  Almost certainly,
we should not back-patch this kind of change.

regards, tom lane

diff --git a/src/backend/utils/mb/conversion_procs/cyrillic_and_mic/cyrillic_and_mic.c b/src/backend/utils/mb/conversion_procs/cyrillic_and_mic/cyrillic_and_mic.c
index 5d1c59b..97e890d 100644
*** a/src/backend/utils/mb/conversion_procs/cyrillic_and_mic/cyrillic_and_mic.c
--- b/src/backend/utils/mb/conversion_procs/cyrillic_and_mic/cyrillic_and_mic.c
*** iso2mic(const unsigned char *l, unsigned
*** 433,439 
  		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
  		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
  		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
! 		0x00, 0xb3, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
  		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
  		0xe1, 0xe2, 0xf7, 0xe7, 0xe4, 0xe5, 0xf6, 0xfa,
  		0xe9, 0xea, 0xeb, 0xec, 0xed, 0xee, 0xef, 0xf0,
--- 433,439 
  		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
  		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
  		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
! 		0x9a, 0xb3, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
  		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
  		0xe1, 0xe2, 0xf7, 0xe7, 0xe4, 0xe5, 0xf6, 0xfa,
  		0xe9, 0xea, 0xeb, 0xec, 0xed, 0xee, 0xef, 0xf0,
*** mic2iso(const unsigned char *mic, unsign
*** 458,464 
  		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
  		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
  		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
! 		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
  		0x00, 0x00, 0x00, 0xf1, 0x00, 0x00, 0x00, 0x00,
  		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
  		0x00, 0x00, 0x00, 0xa1, 0x00, 0x00, 0x00, 0x00,
--- 458,464 
  		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
  		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
  		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
! 		0x00, 0x00, 0xa0, 0x00, 0x00, 0x00, 0x00, 0x00,
  		0x00, 0x00, 0x00, 0xf1, 0x00, 0x00, 0x00, 0x00,
  		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
  		0x00, 0x00, 0x00, 0xa1, 0x00, 0x00, 0x00, 0x00,
*** win12512mic(const unsigned char *l, unsi
*** 485,494 
  		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
  		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
  		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
! 		0x00, 0x00, 0x00, 0x00, 0x00, 0xbd, 0x00, 0x00,
! 		0xb3, 0x00, 0xb4, 0x00, 0x00, 0x00, 0x00, 0xb7,
! 		0x00, 0x00, 0xb6, 0xa6, 0xad, 0x00, 0x00, 0x00,
! 		0xa3, 0x00, 0xa4, 0x00, 0x00, 0x00, 0x00, 0xa7,
  		0xe1, 0xe2, 0xf7, 0xe7, 0xe4, 0xe5, 0xf6, 0xfa,
  		0xe9, 0xea, 0xeb, 0xec, 0xed, 0xee, 0xef, 0xf0,
  		0xf2, 0xf3, 0xf4, 0xf5, 0xe6, 0xe8, 0xe3, 0xfe,
--- 485,494 
  		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
  		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
  		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
! 		0x9a, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
! 		0xb3, 0xbf, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
! 		0x9c, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x9e,
! 		0xa3, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
  		0xe1, 0xe2, 0xf7, 0xe7, 0xe4, 0xe5, 0xf6, 0xfa,
  		0xe9, 0xea, 0xeb, 0xec, 0xed, 0xee, 0xef, 0xf0,
  		0xf2, 0xf3, 0xf4, 0xf5, 0xe6, 0xe8, 0xe3, 0xfe,
*** mic2win1251(const unsigned char *mic, un
*** 510,520 
  		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
  		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
  		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
  		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 

Re: [HACKERS] New email address

[Greg Stark]

On Tue, Nov 24, 2015 at 7:50 PM, Tom Lane  wrote:
> Yeah, but SPF is also used as part of DMARC, which means that merely
> forwarding somebody's email out of our listserv is probably going to look
> like spam, even if we didn't change anything at all about the message
> contents.  Also, some sources sign Reply-To: and/or Sender: (Yahoo,
> at least, does the former) which means you can't replace those headers
> either without breaking the DKIM signature.  The only fix for that is to
> rewrite From:, and once you do that I don't see a convincing argument why
> you can't also rewrite Subject: and add a footer if you feel like it.


>From what I'm reading in DMARC if the DKIM signature is valid then the
email should be accepted regardless of SPF. Many of the sources I'm
reading say that the p=reject Yahoo change only affected lists that
break DKIM signatures. If that's the case then I would say avoiding
breaking DKIM is by far the preferable solution.

Fwiw I think people are by far underestimating the breakage that
rewriting From headers will produce. It breaks assumptions in lots of
places. Just think, you can no longer search for or filter emails from
someone reliably, autoresponders will now respond to the list, if you
Cc the mail then which From header the recipient sees is random, and
so on and so on. I can't imagine the mayhem that cross-posting will
produce. It's just a terrible terrible idea.

-- 
greg


-- 
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] New email address

[José Luis Tallón]

On 11/26/2015 09:12 PM, Greg Stark wrote:

On Wed, Nov 25, 2015 at 6:55 PM, Tom Lane  wrote:

But my point was that while the RFC says what to put there there's
absolutely no reference anywhere for when the information should cause
any MUA or MTA to behave differently.

Agreed.  To my mind that's a reason why Sender should not be DKIM-signed.
Unfortunately, RFC 6376 explicitly suggests doing so ... and it looks like
some people are taking that advice.

Hm, I see it as a reason why signing Sender is reasonable. If it were
a functional header then there might be a reason it would have to be
changed. But if it's purely informational and the receiving MUA is
going to display to the user (which is a bad idea imho but Gmail and
Exchange both do it) then it makes sense to expect some authentication
for it. I think the thinking is basically "sign everything we're going
to present to the user phishers can't claim to be someone they're
not". In which case it's fairly important that things like Sender be
signed. Or that everyone agree it's just a useless header and stop
sending or displaying it.


From DMARC.org's Wiki:
<<< 2 Add an "Original Authentication Results" header to indicate you have
performed the authentication and you are validating it
3 Take ownership of the email, by removing the DKIM signature and 
putting your own
as well as changing the from header in the email to contain an email 
address

within your mailing list domain. >>>


Read elsewhere: "To allow for forwarding scenarios, DMARC also allows 
the *Display From* to be cryptographically signed by DKIM, and if any 
unauthorized spammer or phisher were to attempt to assume that identity, 
the encryption would fail."



I don't think we should base any action on guesses of what Gmail does.
Google may do something we don't expect that's more complex to work
around the problem. For one thing you can have email addresses at
Google from a number of domains so they may well be able to have more
than one policy for different users.

Yep

I would suggest we stop doing things that are obviously incompatible
with DKIM -- header and body munging for example. And I suspect we can
stop touching Sender without any ill effects too.

One idea might be to add a script to check a user's domain for
p=reject and send them a warning when subscribing (or sending mail to
the list?) warning them of the problem.

Definitively worth the effort, unless an almost perfect solution is found :S


/ J.L.

Re: [HACKERS] New email address

[Alvaro Herrera]

Greg Stark wrote:

> Hm, I see it as a reason why signing Sender is reasonable. If it were
> a functional header then there might be a reason it would have to be
> changed. But if it's purely informational and the receiving MUA is
> going to display to the user (which is a bad idea imho but Gmail and
> Exchange both do it) then it makes sense to expect some authentication
> for it. I think the thinking is basically "sign everything we're going
> to present to the user phishers can't claim to be someone they're
> not". In which case it's fairly important that things like Sender be
> signed. Or that everyone agree it's just a useless header and stop
> sending or displaying it.

As the recipient of most -owner addresses, I would be glad to stop
munging Sender.  For some reason, some mailers record that as the
address of the mailing list in the user's addressbook; so if in the
future they send emails to the list, they end up in my mailbox instead
of posted.

> One idea might be to add a script to check a user's domain for
> p=reject and send them a warning when subscribing (or sending mail to
> the list?) warning them of the problem.

I don't think that's going to be anything but unwelcome noise.  What
would they do if they became aware of the issue?  They could switch
providers, but that only works for so long.  As soon as Gmail switches
to p=reject, we've lost.  We got away with doing it for Yahoo because
there's not a lot of people using that -- not on these lists anyway.

-- 
Á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] New email address

[Greg Stark]

On Thu, Nov 26, 2015 at 11:13 PM, José Luis Tallón
 wrote:
> From DMARC.org's Wiki:
> <<< 2 Add an "Original Authentication Results" header to indicate you have
> performed the authentication and you are validating it
> 3 Take ownership of the email, by removing the DKIM signature and putting
> your own
> as well as changing the from header in the email to contain an email address
> within your mailing list domain. >>>


But you missed option #1:

1. Operate strictly as a "forwarder," where the RFC5321.RcptTo field
is changed to send the message to list members, but the RFC5322
message headers and body are not altered.
Pros: Receiving systems can validate the DKIM signature of the message
author, if one was present.
Cons: Senders that depend solely on SPF for authentication will still
fail. Precludes many customary features of mailing lists, such as
"Subject:" tags, list footers/disclaimers, etc.


-- 
greg


-- 
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] Errors in our encoding conversion tables

[Tatsuo Ishii]

> There's a discussion over at
> http://www.postgresql.org/message-id/flat/2sa.dhu5.1hk1yrptnfy.1ml...@seznam.cz
> of an apparent error in our WIN1250 -> LATIN2 conversion.  I looked into this
> and found that indeed, the code will happily translate certain characters
> for which there seems to be no justification.  I made up a quick script
> that would recompute the conversion tables in latin2_and_win1250.c from
> the Unicode mapping files in src/backend/utils/mb/Unicode, and what it
> computes is shown in the attached diff.  (Zeroes in the tables indicate
> codes with no translation, for which an error should be thrown.)
> 
> Having done that, I thought it would be a good idea to see if we had any
> other conversion tables that weren't directly based on the Unicode data.
> The only ones I could find were in cyrillic_and_mic.c, and those seem to
> be absolutely filled with errors, to the point where I wonder if they were
> made from the claimed encodings or some other ones.  The attached patch
> recomputes those from the Unicode data, too.
> 
> None of this data seems to have been touched since Tatsuo-san's original
> commit 969e0246, so it looks like we simply didn't vet that submission
> closely enough.
> 
> I have not attempted to reverify the files in utils/mb/Unicode against the
> original Unicode Consortium data, but maybe we ought to do that before
> taking any further steps here.
> 
> Anyway, what are we going to do about this?  I'm concerned that simply
> shoving in corrections may cause problems for users.  Almost certainly,
> we should not back-patch this kind of change.

I have started to looking into it. I wonder how do you create the part
of your patch:

*** 154,163 
  win12502mic(const unsigned char *l, unsigned char *p, int len)
  {
static const unsigned char win1250_2_iso88592[] = {
!   0x80, 0x81, 0x82, 0x83, 0x84, 0x85, 0x86, 0x87,
!   0x88, 0x89, 0xA9, 0x8B, 0xA6, 0xAB, 0xAE, 0xAC,
!   0x90, 0x91, 0x92, 0x93, 0x94, 0x95, 0x96, 0x97,
!   0x98, 0x99, 0xB9, 0x9B, 0xB6, 0xBB, 0xBE, 0xBC,
0xA0, 0xB7, 0xA2, 0xA3, 0xA4, 0xA1, 0x00, 0xA7,
0xA8, 0x00, 0xAA, 0x00, 0x00, 0xAD, 0x00, 0xAF,
0xB0, 0x00, 0xB2, 0xB3, 0xB4, 0x00, 0x00, 0x00,
--- 154,163 
  win12502mic(const unsigned char *l, unsigned char *p, int len)
  {
static const unsigned char win1250_2_iso88592[] = {
!   0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
!   0x00, 0x00, 0xA9, 0x00, 0xA6, 0xAB, 0xAE, 0xAC,
!   0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
!   0x00, 0x00, 0xB9, 0x00, 0xB6, 0xBB, 0xBE, 0xBC,
0xA0, 0xB7, 0xA2, 0xA3, 0xA4, 0xA1, 0x00, 0xA7,
0xA8, 0x00, 0xAA, 0x00, 0x00, 0xAD, 0x00, 0xAF,
0xB0, 0x00, 0xB2, 0xB3, 0xB4, 0x00, 0x00, 0x00,

In the above you seem to disable the conversion from 0x96 of win1250
to ISO-8859-2 by using the Unicode mapping files in
src/backend/utils/mb/Unicode. But the corresponding mapping file
(iso8859_2_to_utf8.amp) does include following entry:

  {0x0096, 0xc296},

How do you know 0x96 should be removed from the conversion?

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] Foreign join pushdown vs EvalPlanQual

[Etsuro Fujita]

On 2015/11/27 0:14, Kouhei Kaigai wrote:

>> The documentation says as following so I think the former has.
>>
>> # I don't understhad what 'can or must' means, though... 'can and
>> # must'?
>>
>> + Also, this callback can or must recheck scan qualifiers and join
>> + conditions which are pushed down. Especially, it needs special

> If fdw_recheck_quals is set up correctly and join type is inner join,
> FDW driver does not recheck by itself. Elsewhere, it has to recheck
> the joined tuple, not only reconstruction.

Sorry, I don't understand this.  In my understanding, fdw_recheck_quals
can be defined for a foreign join, regardless of the join type, and when
the fdw_recheck_quals are defined, the RecheckForeignScan callback
routine doesn't need to evaluate the fdw_recheck_quals by itself.  No?

Best regards,
Etsuro Fujita



-- 
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: About CMake v2

[Craig Ringer]

On 27 November 2015 at 12:39, Tom Lane  wrote:

> Euler Taveira  writes:
> > On 26-11-2015 14:06, YUriy Zhuravlev wrote:
> >> I meant that support for older versions of CMake I'll do when will
> implement
> >> other functions.
>
> > I think you don't understand the point: start with the *right* cmake
> > version because you could have to redo (a lot of) your work or have your
> > patch rejected because you don't follow our advice.
>
> The way Yuriy wants to do it is not necessarily wrong.  He might end up
> with cleaner code if he starts by making a cmake-3 implementation and then
> figures out how to back-port to cmake-2, rather than starting with the
> more limited language to begin with.
>
> Or maybe not.  But I doubt it's open and shut.


One thing to consider: I can't imagine backporting this to all supported
back branches, it'd be a switch for the next release. Right?

That means he doesn't have to worry about what RH / Debian policy for their
old versions is. RH isn't going to release PostgreSQL 9.7 or whatever for
RHEL6, Debian isn't going to release it for Wheezy, etc.

We are. Or rather, the people within the community who perform the
thankless task of packaging are.

The people who need to be involved here are the PGDG package maintainers
for apt.postgresql.org and yum.postgresql.org. If you can convince them
that adding CMake 3.x as a build-dependency is acceptable and that it's
worth the pain/hassle required to ensure it's available you might avoid the
backport to CMake 2.8. Especially if someone else has already done the work
to backport CMake 3.x to wheezy, RHEL6 etc and there's a repo we can just
import the packages from or rebuild into PGDG.

A packaged version of the CMake needed will be vital for PGDG packages.
There's around about zero chance you'll get anywhere with the requirement
to hand-compile CMake to build packages. If you can't list it as a
build-depends and have the package fetched from a well-established repo
(PGDG, the distro repo, or distro official backports) I think you'd have to
support the older CMake instead.

Devrim at least has been fairly willing to backport packages and bundle
them into the PGDG repo when they're required to build newer versions of
things like PostGIS. Not without frustration and problems, though. I can't
say anything about the apt repo in this regard.

(I've CC'd Devrim)


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

Re: [HACKERS] WIP: About CMake v2

[Tom Lane]

Craig Ringer  writes:
> One thing to consider: I can't imagine backporting this to all supported
> back branches, it'd be a switch for the next release. Right?

Agreed.

> That means he doesn't have to worry about what RH / Debian policy for their
> old versions is. RH isn't going to release PostgreSQL 9.7 or whatever for
> RHEL6, Debian isn't going to release it for Wheezy, etc.

Well, they won't if we make it impossible for them to do so.

More generally, I do not buy this argument for one second as a reason why
we can demand latest-and-greatest cmake, rather than something that's
likely to be readily available on a wide variety of platforms.  Devrim is
not the only person in the world who will be needing to build PG on RHEL6,
or even older platforms.

If you take a close look at our build requirements, you will notice a
general distaste for insisting on latest anything.  cmake is not going
to escape that project bias.  Do you really think a project that still
works with C89, Perl 5.8.something, Python 2.3, bison 1.875, yadda yadda
is readily going to accept a patch that requires this year's cmake?
It would take a fairly impressive technical argument why working with
older cmakes is impossible/impractical before that will happen.

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] Foreign join pushdown vs EvalPlanQual

[Etsuro Fujita]

On 2015/11/27 0:14, Kouhei Kaigai wrote:

On 2015/11/26 14:04, Kouhei Kaigai wrote:

The attached patch adds: Path *fdw_outerpath field to ForeignPath node.
FDW driver can set arbitrary but one path-node here.
After that, this path-node shall be transformed to plan-node by
createplan.c, then passed to FDW driver using GetForeignPlan callback.



I understand this, as I also did the same thing in my patches, but
actually, that seems a bit complicated to me.  Instead, could we keep
the fdw_outerpath in the fdw_private of a ForeignPath node when creating
the path node during GetForeignPaths, and then create an outerplan
accordingly from the fdw_outerpath stored into the fdw_private during
GetForeignPlan, by using create_plan_recurse there?  I think that that
would make the core involvment much simpler.



How to use create_plan_recurse by extension? It is a static function.


I was just thinking a change to make that function extern.


We expect FDW driver set this plan-node on lefttree (a.k.a outerPlan).
The Plan->outerPlan is a common field, so patch size become relatively
small. FDW driver can initialize this plan at BeginForeignScan, then
execute this sub-plan-tree on demand.



Another idea would be to add the core support for
initializing/closing/rescanning the outerplan tree when the tree is given.



No. Please don't repeat same discussion again.


IIUC, I think your point is to allow FDWs to do something else, instead 
of performing a local join execution plan, during RecheckForeignScan. 
So, what's wrong with the core doing that support in that case?



@@ -85,6 +86,18 @@ ForeignRecheck(ForeignScanState *node, TupleTableSlot
*slot)

ResetExprContext(econtext);

+   /*
+* FDW driver has to recheck visibility of EPQ tuple towards
+* the scan qualifiers once it gets pushed down.
+* In addition, if this node represents a join sub-tree, not
+* a scan, FDW driver is also responsible to reconstruct
+* a joined tuple according to the primitive EPQ tuples.
+*/
+   if (fdwroutine->RecheckForeignScan)
+   {
+   if (!fdwroutine->RecheckForeignScan(node, slot))
+   return false;
+   }

Maybe I'm missing something, but I think we should let FDW do the work
if scanrelid==0, not just if fdwroutine->RecheckForeignScan is given.
(And if scanrelid==0 and fdwroutine->RecheckForeignScan is not given, we
should abort the transaction.)



It should be Assert(). The node with scanrelid==0 never happen
unless FDW driver does not add such a path explicitly.


That's an idea.  But the abort seems to me more consistent with other 
places (see eg, RefetchForeignRow in EvalPlanQualFetchRowMarks).



Another thing I'm concerned about is

@@ -347,8 +355,26 @@ ExecScanReScan(ScanState *node)
{
Index   scanrelid = ((Scan *)
node->ps.plan)->scanrelid;

-   Assert(scanrelid > 0);
+   if (scanrelid > 0)
+   estate->es_epqScanDone[scanrelid - 1] = false;
+   else
+   {
+   Bitmapset  *relids;
+   int rtindex = -1;
+
+   if (IsA(node->ps.plan, ForeignScan))
+   relids = ((ForeignScan *)
node->ps.plan)->fs_relids;
+   else if (IsA(node->ps.plan, CustomScan))
+   relids = ((CustomScan *)
node->ps.plan)->custom_relids;
+   else
+   elog(ERROR, "unexpected scan node: %d",
+(int)nodeTag(node->ps.plan));

-   estate->es_epqScanDone[scanrelid - 1] = false;
+   while ((rtindex = bms_next_member(relids, rtindex)) >=
0)
+   {
+   Assert(rtindex > 0);
+   estate->es_epqScanDone[rtindex - 1] = false;
+   }
+   }
}

That seems the outerplan's business to me, so I think it'd be better to
just return, right before the assertion, as I said before.  Seen from
another angle, ISTM that FDWs that don't use a local join execution plan
wouldn't need to be aware of handling the es_epqScanDone flags.  (Do you
think that such FDWs should do something like what ExecScanFtch is doing
about the flags, in their RecheckForeignScans?  If so, I think we need
docs for that.)



Execution of alternative local subplan (outerplan) is discretional.
We have to pay attention FDW drivers which handles EPQ recheck by
itself. Even though you argue callback can violate state of
es_epqScanDone flags, it is safe to follow the existing behavior.


So, I think the documentation needs more work.

Yet another thing that I'm concerned about is

@@ -3747,7 +3754,8 @@ make_foreignscan(List *qptlist,
 List *fdw_exprs,
 List *fdw_private,
   

Re: [HACKERS] WIP: About CMake v2

[Tom Lane]

Euler Taveira  writes:
> On 26-11-2015 14:06, YUriy Zhuravlev wrote:
>> I meant that support for older versions of CMake I'll do when will implement 
>> other functions.

> I think you don't understand the point: start with the *right* cmake
> version because you could have to redo (a lot of) your work or have your
> patch rejected because you don't follow our advice.

The way Yuriy wants to do it is not necessarily wrong.  He might end up
with cleaner code if he starts by making a cmake-3 implementation and then
figures out how to back-port to cmake-2, rather than starting with the
more limited language to begin with.

Or maybe not.  But I doubt it's open and shut.

regards, tom lane


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

Re: [HACKERS] pg_stat_replication log positions vs base backups

[Michael Paquier]

On Thu, Nov 26, 2015 at 10:53 PM, Magnus Hagander  wrote:
> On Thu, Nov 26, 2015 at 1:03 PM, Michael Paquier 
> wrote:
>>
>> On Thu, Nov 26, 2015 at 6:45 PM, Magnus Hagander wrote:
>> > I'm only talking about the actual value in pg_stat_replication here, not
>> > what we are using internally. These are two different things of course -
>> > let's keep them separate for now. In pg_stat_replication, we explicitly
>> > check for InvalidXLogRecPtr and then explicitly set the resulting value
>> > to
>> > NULL in the SQL return.
>>
>> No objections from here. I guess you already have a patch?
>
> Well, no, because I haven't figured out which way is the logical one - make
> them all return NULL or make them all return 0/0...

It seems to me that NULL is the logical one. We want to define a value
from the user prospective where things are in an undefined state.
That's my logic flow, other opinions are of course welcome.
-- 
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] WIP: About CMake v2

[Michael Paquier]

On Fri, Nov 27, 2015 at 2:50 PM, Craig Ringer  wrote:
> On 27 November 2015 at 12:39, Tom Lane  wrote:
>>
>> Euler Taveira  writes:
>> > On 26-11-2015 14:06, YUriy Zhuravlev wrote:
>> >> I meant that support for older versions of CMake I'll do when will
>> >> implement
>> >> other functions.
>>
>> > I think you don't understand the point: start with the *right* cmake
>> > version because you could have to redo (a lot of) your work or have your
>> > patch rejected because you don't follow our advice.
>>
>> The way Yuriy wants to do it is not necessarily wrong.  He might end up
>> with cleaner code if he starts by making a cmake-3 implementation and then
>> figures out how to back-port to cmake-2, rather than starting with the
>> more limited language to begin with.
>>
>> Or maybe not.  But I doubt it's open and shut.
>
> One thing to consider: I can't imagine backporting this to all supported
> back branches, it'd be a switch for the next release. Right?
>
> That means he doesn't have to worry about what RH / Debian policy for their
> old versions is. RH isn't going to release PostgreSQL 9.7 or whatever for
> RHEL6, Debian isn't going to release it for Wheezy, etc.
>
> We are. Or rather, the people within the community who perform the thankless
> task of packaging are.

That's not only a problem related to packaging by pgdg maintainers or
the main Linux distributions. I think that you are forgetting people
who actually work on or compile the Postgres code on those platforms,
say because they for example need to perform performance test or
measurements particularly on such platforms based on customer
requirements, one possibility being to test different configure
options like bigger relation file blocks for example. It does not seem
acceptable to me to choose a minimal version of cmake without this
criteria taken into account.
-- 
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] Error with index on unlogged table

[Michael Paquier]

On Sat, Nov 21, 2015 at 11:30 PM, Michael Paquier
 wrote:
> On Fri, Nov 20, 2015 at 4:11 PM, Michael Paquier
>  wrote:
>> For master and perhaps 9.5, I would think that a dedicated WAL record
>> type enforcing the fsync is the way to go. This special treatment
>> would go only for btree and spgist when they use INIT_FORKNUM and we
>> would not impact other relation types and code paths with this
>> behavior.
>
> So, I have been looking at that, and finished with the attached to
> implement this idea. This way, it is possible to control at replay if
> a relation should be synced to disk just after replaying a FPI or a
> set of FPIs. This makes btree and spgist init forks more consistent at
> replay with what is done on master by syncing them immediately, which
> is not a bad thing to me.

Depending on the type of index used on an unlogged table, the failure
happening is quite various. With gist and btree, a promoted standby
will complain about an empty page. With brin, the standby will
complain with a floating-point error:
ERROR:  22P01: floating-point exception
DETAIL:  An invalid floating-point operation was signaled. This
probably means an out-of-range result or an invalid operation, such as
division by zero.
LOCATION:  FloatExceptionHandler, postgres.c:2702

Now with gin, the system becomes actually unresponsive, being stuck on
PGSemaphoreLock and unable to answer to signals:
frame #1: 0x00010a29f7cf
postgres`PGSemaphoreLock(sema=0x0001138df118) + 63 at
pg_sema.c:387
frame #2: 0x00010a348e81
postgres`LWLockAcquire(lock=0x00010acc5dc0, mode=LW_EXCLUSIVE) +
369 at lwlock.c:1026
frame #3: 0x00010a3190b1 postgres`LockBuffer(buffer=208,
mode=2) + 289 at bufmgr.c:3240
frame #4: 0x000109f341e1
postgres`ginHeapTupleFastInsert(ginstate=0x7fff55d06a08,
collector=0x7fff55d069d8) + 849 at ginfast.c:309
frame #5: 0x000109f1383f
postgres`gininsert(fcinfo=0x7fff55d09010) + 431 at gininsert.c:531

I am still investigating for a correct fix, looking at reinit.c the
code in charge of copying the init fork as the main fork for a
relation at the end of recovery looks to be doing its job correctly...
Regards,
-- 
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] New email address

[Stefan Kaltenbrunner]

On 11/26/2015 09:10 PM, Tom Lane wrote:
> Stefan Kaltenbrunner  writes:
>> that seems entirely doable with our current infrastructure (and even
>> with minimal-to-no hackery on mj2) - but it still carries the "changes
>> From:" issue :/
> 
> Yeah.  What do you think of the other approach of trying to preserve
> validity of the incoming DKIM-Signature (in particular, by not changing
> the Subject or message body)?

well not changing the subject seems like something we could do without
fuss - not changing the body would likely mean we would (again) get a
number of people asking "how do I unsubscribe", but maybe we will have
to live with that.

As for google/gmail - it seems they are indeed moving towards p=reject
based on:

https://dmarc.org/2015/10/global-mailbox-providers-deploying-dmarc-to-protect-users/
https://wordtothewise.com/2015/10/dmarc-news-gmail-preject-and-arc/


so we have to do "something" anyway (before June 2016) - I have not
actually studied the referenced ietf drafts mentioned in the second post
yet so maybe there is something in there to help with our usecase...


Stefan


-- 
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] Errors in our encoding conversion tables

[Tom Lane]

Tatsuo Ishii  writes:
> I have started to looking into it. I wonder how do you create the part
> of your patch:

The code I used is below.

> In the above you seem to disable the conversion from 0x96 of win1250
> to ISO-8859-2 by using the Unicode mapping files in
> src/backend/utils/mb/Unicode. But the corresponding mapping file
> (iso8859_2_to_utf8.amp) does include following entry:

>   {0x0096, 0xc296},

> How do you know 0x96 should be removed from the conversion?

Right, but there is no mapping in the win1250-utf8 files that matches
U+C296.  The complaint over in the other thread is precisely that we
have no business translating 0x96 in WIN1250 to this character.  What
WIN1250 0x96 could translate to is U+E28093 (at least, according to
win1250_to_utf8.map) but that Unicode character has no equivalent in
LATIN2.

AFAICS, whoever made these tables just arbitrarily decided that 0x96
in WIN1250 could be mapped to 0x96 in LATIN2, and likewise for a number
of other codes; but those are false equivalences, as you find out if
you try to perform the same conversion via other encoding conversion
paths, ie convert to UTF8 and then to the other encoding.

regards, tom lane

#include "c.h"
#include "mb/pg_wchar.h"

#include "src/backend/utils/mb/Unicode/iso8859_2_to_utf8.map"
#include "src/backend/utils/mb/Unicode/iso8859_5_to_utf8.map"
#include "src/backend/utils/mb/Unicode/win1250_to_utf8.map"
#include "src/backend/utils/mb/Unicode/win1251_to_utf8.map"
#include "src/backend/utils/mb/Unicode/win866_to_utf8.map"
#include "src/backend/utils/mb/Unicode/koi8r_to_utf8.map"
#include "src/backend/utils/mb/Unicode/koi8u_to_utf8.map"


typedef struct
{
	const pg_local_to_utf *map1;	/* to UTF8 map name */
	int			size1;			/* size of map1 */
	const pg_local_to_utf *map2;	/* to UTF8 map name */
	int			size2;			/* size of map2 */
	const char *tabname;
	int			upper;
} pg_conv_map;

static const pg_conv_map maps[] = {
	{
		LUmapWIN1250, lengthof(LUmapWIN1250),
		LUmapISO8859_2, lengthof(LUmapISO8859_2),
		"win1250_2_iso88592", 1
	},
	{
		LUmapISO8859_2, lengthof(LUmapISO8859_2),
		LUmapWIN1250, lengthof(LUmapWIN1250),
		"iso88592_2_win1250", 1
	},
	{
		LUmapISO8859_5, lengthof(LUmapISO8859_5),
		LUmapKOI8R, lengthof(LUmapKOI8R),
		"iso2koi", 0
	},
	{
		LUmapKOI8R, lengthof(LUmapKOI8R),
		LUmapISO8859_5, lengthof(LUmapISO8859_5),
		"koi2iso", 0
	},
	{
		LUmapWIN1251, lengthof(LUmapWIN1251),
		LUmapKOI8R, lengthof(LUmapKOI8R),
		"win2koi", 0
	},
	{
		LUmapKOI8R, lengthof(LUmapKOI8R),
		LUmapWIN1251, lengthof(LUmapWIN1251),
		"koi2win", 0
	},
	{
		LUmapWIN866, lengthof(LUmapWIN866),
		LUmapKOI8R, lengthof(LUmapKOI8R),
		"win8662koi", 0
	},
	{
		LUmapKOI8R, lengthof(LUmapKOI8R),
		LUmapWIN866, lengthof(LUmapWIN866),
		"koi2win866", 0
	},

};

static void
domap(const pg_conv_map *info)
{
	uint32 c;

	printf("	static const unsigned char %s[] = {\n", info->tabname);

	for (c = 0x80; c <= 0xff; c++)
	{
		uint32 u = 0;
		uint32 c2 = 0;
		int i;

		for (i = 0; i < info->size1; i++)
		{
			if (info->map1[i].code == c)
			{
u = info->map1[i].utf;
break;
			}
		}
		if (u != 0)
		{
			for (i = 0; i < info->size2; i++)
			{
if (info->map2[i].utf == u)
{
	c2 = info->map2[i].code;
	break;
}
			}
		}
#if 0
		if (c2)
			printf("0x%02x maps to 0x%02x via U+%04X\n", c, c2, u);
		else
			printf("0x%02x has no equivalent\n", c);
#endif
		if (c % 8 == 0)
			printf("\t\t");
		if (info->upper)
			printf("0x%02X", c2);
		else
			printf("0x%02x", c2);
		if (c == 0xff)
			printf("\n");
		else if (c % 8 == 7)
			printf(",\n");
		else
			printf(", ");
	}
	printf("\t};\n\n");
}

int
main()
{
	int i;

	for (i = 0; i < lengthof(maps); i++)
		domap(maps + i);

	return 0;
}

-- 
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] Foreign join pushdown vs EvalPlanQual

[Kouhei Kaigai]

> -Original Message-
> From: Etsuro Fujita [mailto:fujita.ets...@lab.ntt.co.jp]
> Sent: Friday, November 27, 2015 2:40 PM
> To: Kaigai Kouhei(海外 浩平); Kyotaro HORIGUCHI
> Cc: robertmh...@gmail.com; t...@sss.pgh.pa.us; pgsql-hackers@postgresql.org;
> shigeru.han...@gmail.com
> Subject: Re: [HACKERS] Foreign join pushdown vs EvalPlanQual
> 
> On 2015/11/27 0:14, Kouhei Kaigai wrote:
> 
> >> The documentation says as following so I think the former has.
> >>
> >> # I don't understhad what 'can or must' means, though... 'can and
> >> # must'?
> >>
> >> + Also, this callback can or must recheck scan qualifiers and join
> >> + conditions which are pushed down. Especially, it needs special
> 
> > If fdw_recheck_quals is set up correctly and join type is inner join,
> > FDW driver does not recheck by itself. Elsewhere, it has to recheck
> > the joined tuple, not only reconstruction.
> 
> Sorry, I don't understand this.  In my understanding, fdw_recheck_quals
> can be defined for a foreign join, regardless of the join type,
>
Yes, "can be defined", but will not be workable if either side of
joined tuple is NULL because of outer join. SQL functions returns
NULL prior to evaluation, then ExecQual() treats this result as FALSE.
However, a joined tuple that has NULL fields may be a valid tuple.

We don't need to care about unmatched tuple if INNER JOIN.

> and when
> the fdw_recheck_quals are defined, the RecheckForeignScan callback
> routine doesn't need to evaluate the fdw_recheck_quals by itself.  No?
>
Yes, it does not need to run fdw_recheck_quals by itself (if they
can guarantee correct results for any corner cases).
Of course, if FDW driver keep expression for scan-qualifiers and
join-clauses on another place (like fdw_exprs), it is FDW driver's
responsibility to execute it, regardless of fdw_recheck_quals.

Thanks,
--
NEC Business Creation Division / PG-Strom Project
KaiGai Kohei 



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

Re: [HACKERS] parallel joins, and better parallel explain

[Simon Riggs]

On 26 November 2015 at 03:41, Robert Haas  wrote:

> Attached find a patch that does (mostly) two things.  First, it allows
> the optimizer to generate plans where a Nested Loop or Hash Join
> appears below a Gather node.  This is a big improvement on what we
> have today, where only a sequential scan can be parallelized; with
> this patch, entire join problems can be parallelized, as long as they
> don't need a Merge Join (see below for more on this).


Sounds like good progress.

This gives us multiple copies of the hash table, which means we must either
use N * work_mem, or we must limit the hash table to work_mem / N per
partial plan.

How can the partial paths share a hash table?

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

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

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

[Etsuro Fujita]

Hi Thom,

Thank you for paying attention to this!

On 2015/11/25 20:36, Thom Brown wrote:

On 13 May 2015 at 04:10, Etsuro Fujita  wrote:

On 2015/05/13 0:55, Stephen Frost wrote:

While the EXPLAIN output changed, the structure hasn't really changed
from what was discussed previously and there's not been any real
involvment from the core code in what's happening here.

Clearly, the documentation around how to use the FDW API hasn't changed
at all and there's been no additions to it for handling bulk work.
Everything here continues to be done inside of postgres_fdw, which
essentially ignores the prescribed "Update/Delete one tuple" interface
for ExecForeignUpdate/ExecForeignDelete.

I've spent the better part of the past two days trying to reason my way
around that while reviewing this patch and I haven't come out the other
side any happier with this approach than I was back in
20140911153049.gc16...@tamriel.snowman.net.

There are other things that don't look right to me, such as what's going
on at the bottom of push_update_down(), but I don't think there's much
point going into it until we figure out what the core FDW API here
should look like.  It might not be all that far from what we have now,
but I don't think we can just ignore the existing, documented, API.



OK, I'll try to introduce the core FDW API for this (and make changes to the
core code) to address your previous comments.



I'm a bit behind in reading up on this, so maybe it's been covered
since, but is there a discussion of this API on another thread, or a
newer patch available?


Actually, I'm now working on this.  My basic idea is to add new FDW APIs 
for handling the bulk work, in order not to make messy the prescribed 
"Update/Delete one tuple" interface; 1) add to nodeModifyTable.c or 
nodeForeignscan.c the new FDW APIs BeginPushedDownForeignModify, 
ExecPushedDownForeignModify and EndPushedDownForeignModify for that, and 
2) call these FDW APIs, instead of BeginForeignModify, 
ExecForeignUpdate/ExecForeignDelete and EndForeignModify, when doing 
update pushdown.  I'd like to propose that in more detail as soon as 
possible, probably with an updated patch.


Best regards,
Etsuro Fujita



--
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_stat_replication log positions vs base backups

[Magnus Hagander]

On Thu, Nov 26, 2015 at 8:17 AM, Michael Paquier 
wrote:

>
>
> On Wed, Nov 25, 2015 at 11:08 PM, Magnus Hagander 
> wrote:
>
>> On Wed, Nov 25, 2015 at 3:03 PM, Michael Paquier <
>> michael.paqu...@gmail.com> wrote:
>>
>>> On Wed, Nov 25, 2015 at 7:18 PM, Magnus Hagander 
>>> wrote:
>>>
>> In particular, it seems that in InitWalSenderSlot, we only initialize the
 sent location. Perhaps this is needed?

>>>
>>> Yes, that would be nice to start with a clean state. At the same time, I
>>> am noticing that pg_stat_get_wal_senders() is comparing flush, apply and
>>> write directly with 0. I think those should be InvalidXLogRecPtr. Combined
>>> with your patch it gives the attached.
>>>
>>
>> Good point.
>>
>> Another thought - why do we leave 0/0 in sent location, but turn the
>> other three fields to NULL if it's invalid? That seems inconsistent. Is
>> there a reason for that, or should we fix that as well while we're at it?
>>
>
> In some code paths, values are expected to be equal to 0 as XLogRecPtr
> values are doing direct comparisons with each other, so you can think that
> 0 for a XLogRecPtr is slightly different from InvalidXLogRecPtr that could
> be expected to be invalid but *not* minimal, imagine for example that we
> decide at some point to redefine it as INT64_MAX. See for example
> receivedUpto in xlog.c or WalSndWaitForWal in walsender.c. Honestly, I
> think that it would be nice to make all the logic consistent under a unique
> InvalidXLogRecPtr banner that is defined at 0 once and for all, adding at
> the same time a comment in xlogdefs.h mentioning that this should not be
> redefined to a higher value because comparison logic of XlogRecPtr's depend
> on that. We have actually a similar constraint with TimeLineID = 0 that is
> equivalent to infinite. Patch attached following those lines, which should
> be backpatched for correctness.
>

I'm only talking about the actual value in pg_stat_replication here, not
what we are using internally. These are two different things of course -
let's keep them separate for now. In pg_stat_replication, we explicitly
check for InvalidXLogRecPtr and then explicitly set the resulting value to
NULL in the SQL return.

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

Re: [HACKERS] Redefine default result from PQhost()?

[Magnus Hagander]

On Wed, Nov 25, 2015 at 11:59 PM, Tom Lane  wrote:

> Currently, libpq's PQhost() function will return NULL if the connection is
> using the default Unix-socket connection address.  This seems like a
> pretty dubious definition to me.  psql works around it in several places
> with
>
> host = PQhost(pset.db);
> if (host == NULL)
> host = DEFAULT_PGSOCKET_DIR;
>
> That seems rather duplicative, and it also means that both libpq and psql
> have the default socket directory hard-wired into them, which does not
> seem like a good thing.  It's conceivable that psql could get used with
> a libpq built with a different default, in which case these places would
> be outright lying about which socket is being used.
>
> I think we should do this:
>
>  char *
>  PQhost(const PGconn *conn)
>  {
>  if (!conn)
>  return NULL;
>  if (conn->pghost != NULL && conn->pghost[0] != '\0')
>  return conn->pghost;
>  else
>  {
>  #ifdef HAVE_UNIX_SOCKETS
> -return conn->pgunixsocket;
> +if (conn->pgunixsocket != NULL && conn->pgunixsocket[0] != '\0')
> +return conn->pgunixsocket;
> +else
> +return DEFAULT_PGSOCKET_DIR;
>  #else
>  return DefaultHost;
>  #endif
>  }
>  }
>
> As a definitional change, this would be for HEAD only.
>
> Comments?
>

I agree with this change in genera. But I wonder if there's a risk here
that we break some applications isnt' it? It's clearly a backwards
incompatible change, so wouldn't it require a bump of libpq version? And
I'm not sure it's worth that on it's own...

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

Re: CustomScan in a larger structure (RE: [HACKERS] CustomScan support on readfuncs.c)

[Kouhei Kaigai]

> -Original Message-
> From: pgsql-hackers-ow...@postgresql.org
> [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Kouhei Kaigai
> Sent: Saturday, November 21, 2015 8:05 AM
> To: Robert Haas
> Cc: Andres Freund; Amit Kapila; pgsql-hackers
> Subject: Re: CustomScan in a larger structure (RE: [HACKERS] CustomScan 
> support
> on readfuncs.c)
> 
> > On Thu, Nov 19, 2015 at 10:11 PM, Kouhei Kaigai  
> > wrote:
> > > The above two points are for the case if and when extension want to use
> > > variable length fields for its private fields.
> > > So, nodeAlloc() callback is not a perfect answer for the use case because
> > > length of the variable length fields shall be (usually) determined by the
> > > value of another fields (like num_inner_rels, num_gpu_devices, ...) thus
> > > we cannot determine the correct length before read.
> > >
> > > Let's assume an custom-scan extension that wants to have:
> > >
> > >   typedef struct {
> > >   CustomScancscan;
> > >   int   priv_value_1;
> > >   long  priv_value_2;
> > >   extra_info_t  extra_subplan_info[FLEXIBLE_ARRAY_MEMBER];
> > >   /* its length equal to number of sub-plans */
> > >   } ExampleScan;
> > >
> > > The "extnodename" within CustomScan allows to pull callback functions
> > > to handle read node from the serialized format.
> > > However, nodeAlloc() callback cannot determine the correct length
> > > (= number of sub-plans in this example) prior to read 'cscan' part.
> > >
> > > So, I'd like to suggest _readCustomScan (and other extendable nodes
> > > also) read tokens on local CustomScan variable once, then call
> > >   Node *(nodeRead)(Node *local_node)
> > > to allocate entire ExampleScan node and read other private fields.
> > >
> > > The local_node is already filled up by the core backend prior to
> > > the callback invocation, so extension can know how many sub-plans
> > > are expected thus how many private tokens shall appear.
> > > It also means extension can know exact length of the ExampleScan
> > > node, so it can allocate the node as expected then fill up
> > > remaining private tokens.
> >
> > On second thought, I think we should insist that nodes have to be
> > fixed-size.  This sounds like a mess.  If the node needs a variable
> > amount of storage for something, it can store a pointer to a
> > separately-allocated array someplace inside of it.  That's what
> > existing nodes do, and it works fine.
> >
> OK, let's have "node_size" instead of nodeAlloc callback and other
> stuffs.
> 
> Please wait for a patch.
>
I'm now implementing. The above design perfectly works on ForeignScan.
On the other hands, I'd like to have deeper consideration for CustomScan.

My recent patch adds LibraryName and SymbolName on CustomScanMethods
to lookup the method table even if library is not loaded yet.
However, this ExtensibleNodeMethods relies custom scan provider shall
be loaded, by parallel infrastructure, prior to the deserialization.
It means extension has a chance to register itself as well.

My idea is, redefine CustomScanMethod as follows:

typedef struct ExtensibleNodeMethods
{
const char *extnodename;
Sizenode_size;
Node *(*nodeCopy)(const Node *from);
bool  (*nodeEqual)(const Node *a, const Node *b);
void  (*nodeOut)(struct StringInfoData *str, const Node *node);
void  (*nodeRead)(Node *node);
} ExtensibleNodeMethods;

typedef struct CustomScanMethods
{
union {
const char *CustomName;
ExtensibleNodeMethods  xnode;
};
/* Create execution state (CustomScanState) from a CustomScan plan node */
Node   *(*CreateCustomScanState) (struct CustomScan *cscan);
} CustomScanMethods;

It allows to pull CustomScanMethods using GetExtensibleNodeMethods
by CustomName; that is alias of extnodename in ExtensibleNodeMethods.
Thus, we don't need to care about LibraryName and SymbolName.

This kind of trick is not needed for ForeignScan because all the pointer
stuff are reproducible by catalog, however, method table of CustomScan
is not.

How about your opinion?

Thanks,
--
NEC Business Creation Division / PG-Strom Project
KaiGai Kohei 


-- 
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: About CMake v2

[YUriy Zhuravlev]

On Thursday 26 November 2015 01:29:37 Euler Taveira wrote:
> I give it a try. Nice WIP. IMHO you should try to support cmake version
> that are available in the stable releases. Looking at [1], I think the
> best choice is 2.8.11 (because it will cover Red Hat based distros and
> also Debian based ones). Are you using a new feature from 3.1? I mean,
> it should be nice to cover old stable releases, if it is possible.
Maybe you are right. But by the time I finish my work I think 3.0 will become 
a standard. CMake is developing rapidly and soon will have version 3.4.1
And one more thing: a normal documentation came with 3.0. :)
But I try to check my code for 2.8.11, now I have 3.4.0 (latest for Gentoo).
Thanks. 

-- 
YUriy Zhuravlev
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] pg_stat_replication log positions vs base backups

[Michael Paquier]

On Thu, Nov 26, 2015 at 6:45 PM, Magnus Hagander wrote:
> I'm only talking about the actual value in pg_stat_replication here, not
> what we are using internally. These are two different things of course -
> let's keep them separate for now. In pg_stat_replication, we explicitly
> check for InvalidXLogRecPtr and then explicitly set the resulting value to
> NULL in the SQL return.

No objections from here. I guess you already have a 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] Some questions about the array.

[Teodor Sigaev]

YUriy Zhuravlev wrote:

On Friday 06 November 2015 12:55:44 you wrote:

Omitted bounds are common in other languages and would be handy. I
don't think they'd cause any issues with multi-dimensional arrays or
variable start-pos arrays.


And yet, what about my patch?


My vote: let us do it, mean, omitting bounds. It simplifies syntax in rather 
popular queries.




Discussions about ~ and{:} it seems optional.
~ is allowed as unary operator and therefore such syntax will introduce 
incompatibily/ambiguity.



--
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] Getting sorted data from foreign server for merge join

[Rushabh Lathia]

Hi Ashutosh,

I reviewed your latest version of patch and over all the implementation
and other details look good to me.

Here are few cosmetic issues which I found:

1) Patch not getting applied cleanly - white space warning

2)

-List   *usable_pathkeys = NIL;
+List*useful_pathkeys_list = NIL;/* List of all pathkeys */

Code alignment is not correct with other declared variables.

3)

+{
+PathKey*pathkey;
+List*pathkeys;
+
+pathkey = make_canonical_pathkey(root, cur_ec,
+
linitial_oid(cur_ec->ec_opfamilies),
+BTLessStrategyNumber,
+false);
+pathkeys = list_make1(pathkey);
+useful_pathkeys_list = lappend(useful_pathkeys_list, pathkeys);
+}

Code alignment need to fix at make_canonical_pathkey().

4)

I don't understand the meaning of following added testcase into
postgres_fdw.

+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -171,20 +171,35 @@ SELECT COUNT(*) FROM ft1 t1;
 -- join two tables
 SELECT t1.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1) ORDER BY t1.c3,
t1.c1 OFFSET 100 LIMIT 10;
 -- subquery
 SELECT * FROM ft1 t1 WHERE t1.c3 IN (SELECT c3 FROM ft2 t2 WHERE c1 <= 10)
ORDER BY c1;
 -- subquery+MAX
 SELECT * FROM ft1 t1 WHERE t1.c3 = (SELECT MAX(c3) FROM ft2 t2) ORDER BY
c1;
 -- used in CTE
 WITH t1 AS (SELECT * FROM ft1 WHERE c1 <= 10) SELECT t2.c1, t2.c2, t2.c3,
t2.c4 FROM t1, ft2 t2 WHERE t1.c1 = t2.c1 ORDER BY t1.c1;
 -- fixed values
 SELECT 'fixed', NULL FROM ft1 t1 WHERE c1 = 1;
+-- getting data sorted from the foreign table for merge join
+-- Since we are interested in merge join, disable other joins
+SET enable_hashjoin TO false;
+SET enable_nestloop TO false;
+-- inner join, expressions in the clauses appear in the equivalence class
list
+EXPLAIN (VERBOSE, COSTS false)
+SELECT t1.c1, t2."C 1" FROM ft2 t1 JOIN "S 1"."T 1" t2 ON (t1.c1 =
t2."C 1") ORDER BY t1.c1 OFFSET 100 LIMIT 10;
+SELECT t1.c1, t2."C 1" FROM ft2 t1 JOIN "S 1"."T 1" t2 ON (t1.c1 = t2."C
1") ORDER BY t1.c1 OFFSET 100 LIMIT 10;
+-- outer join, expression in the clauses do not appear in equivalence
class list
+-- but no output change as compared to the previous query
+EXPLAIN (VERBOSE, COSTS false)
+SELECT t1.c1, t2."C 1" FROM ft2 t1 LEFT JOIN "S 1"."T 1" t2 ON (t1.c1
= t2."C 1") ORDER BY t1.c1 OFFSET 100 LIMIT 10;
+SELECT t1.c1, t2."C 1" FROM ft2 t1 LEFT JOIN "S 1"."T 1" t2 ON (t1.c1 =
t2."C 1") ORDER BY t1.c1 OFFSET 100 LIMIT 10;
+SET enable_hashjoin TO true;
+SET enable_nestloop TO true;

Because, I removed the code changes of the patch and then I run the test
seem like it has nothing to do with the code changes. Above set of test
giving
same result with/without patch.

Am I missing something ?

Apart from this I debugged the patch for each scenario (query pathkeys and
pathkeys arising out of the equivalence classes) and so far patch looks good
to me.

Attaching update version of patch by fixing the cosmetic changes.


On Fri, Nov 20, 2015 at 9:14 PM, Ashutosh Bapat <
ashutosh.ba...@enterprisedb.com> wrote:

>
>
> On Thu, Nov 19, 2015 at 7:30 AM, Robert Haas 
> wrote:
>
>> On Tue, Nov 17, 2015 at 8:33 AM, Ashutosh Bapat
>>  wrote:
>> >> Although I'm usually on the side of marking things as extern whenever
>> >> we find it convenient, I'm nervous about doing that to
>> >> make_canonical_pathkey(), because it has side effects.  Searching the
>> >> list of canonical pathkeys for the one we need is reasonable, but is
>> >> it really right to ever think that we might create a new one at this
>> >> stage?  Maybe it is, but if so I'd like to hear a good explanation as
>> >> to why.
>> >
>> > For a foreign table no pathkeys are created before creating paths for
>> > individual foreign table since there are no indexes. For a regular
>> table,
>> > the pathkeys get created for useful indexes.
>>
>> OK.
>>
>> Could some portion of the second foreach loop inside
>> get_useful_pathkeys_for_relation be replaced by a call to
>> eclass_useful_for_merging?  The logic looks very similar.
>>
>
> Yes. I have incorporated this change in the patch attached.
>
>
>>
>> More broadly, I'm wondering about the asymmetries between this code
>> and pathkeys_useful_for_merging().  The latter has a
>> right_merge_direction() test and a case for non-EC-derivable join
>> clauses that aren't present in your code.
>
> I wonder if we could just
>> generate pathkeys speculatively and then test which ones survive
>> truncate_useless_pathkeys(), or something like that.  This isn't an
>> enormously complicated patch, but it duplicates more of the logic in
>> pathkeys.c than I'd like.
>>
>
>
> pathkeys_useful_for_merging(), truncate_useless_pathkeys() and host of
> functions in that area are crafted to assess the usefulness of given
> pathkeys which for regular tables are "speculated" from indexes on 

Re: [HACKERS] Foreign join pushdown vs EvalPlanQual

[Etsuro Fujita]

On 2015/11/26 14:04, Kouhei Kaigai wrote:

On 2015/11/24 2:41, Robert Haas wrote:

On Fri, Nov 20, 2015 at 12:11 AM, Kouhei Kaigai  wrote:

One subplan means FDW driver run an entire join sub-tree with local
alternative sub-plan; that is my expectation for the majority case.



What I'm imagining is that we'd add handling that allows the
ForeignScan to have inner and outer children.  If the FDW wants to
delegate the EvalPlanQual handling to a local plan, it can use the
outer child for that.  Or the inner one, if it likes.  The other one
is available for some other purposes which we can't imagine yet.  If
this is too weird, we can only add handling for an outer subplan and
forget about having an inner subplan for now.  I just thought to make
it symmetric, since outer and inner subplans are pretty deeply baked
into the structure of the system.



I'd vote for only allowing an outer subplan.



The attached patch adds: Path *fdw_outerpath field to ForeignPath node.
FDW driver can set arbitrary but one path-node here.
After that, this path-node shall be transformed to plan-node by
createplan.c, then passed to FDW driver using GetForeignPlan callback.


I understand this, as I also did the same thing in my patches, but 
actually, that seems a bit complicated to me.  Instead, could we keep 
the fdw_outerpath in the fdw_private of a ForeignPath node when creating 
the path node during GetForeignPaths, and then create an outerplan 
accordingly from the fdw_outerpath stored into the fdw_private during 
GetForeignPlan, by using create_plan_recurse there?  I think that that 
would make the core involvment much simpler.



We expect FDW driver set this plan-node on lefttree (a.k.a outerPlan).
The Plan->outerPlan is a common field, so patch size become relatively
small. FDW driver can initialize this plan at BeginForeignScan, then
execute this sub-plan-tree on demand.


Another idea would be to add the core support for 
initializing/closing/rescanning the outerplan tree when the tree is given.



Remaining portions are as previous version. ExecScanFetch is revised
to call recheckMtd always when scanrelid==0, then FDW driver can get
control using RecheckForeignScan callback.
It allows FDW driver to handle (1) EPQ recheck on underlying scan nodes,
(2) reconstruction of joined tuple, and (3) EPQ recheck on join clauses,
by its preferable implementation - including execution of an alternative
sub-plan.


@@ -85,6 +86,18 @@ ForeignRecheck(ForeignScanState *node, TupleTableSlot 
*slot)


ResetExprContext(econtext);

+   /*
+* FDW driver has to recheck visibility of EPQ tuple towards
+* the scan qualifiers once it gets pushed down.
+* In addition, if this node represents a join sub-tree, not
+* a scan, FDW driver is also responsible to reconstruct
+* a joined tuple according to the primitive EPQ tuples.
+*/
+   if (fdwroutine->RecheckForeignScan)
+   {
+   if (!fdwroutine->RecheckForeignScan(node, slot))
+   return false;
+   }

Maybe I'm missing something, but I think we should let FDW do the work 
if scanrelid==0, not just if fdwroutine->RecheckForeignScan is given. 
(And if scanrelid==0 and fdwroutine->RecheckForeignScan is not given, we 
should abort the transaction.)


Another thing I'm concerned about is

@@ -347,8 +355,26 @@ ExecScanReScan(ScanState *node)
{
Index   scanrelid = ((Scan *) node->ps.plan)->scanrelid;

-   Assert(scanrelid > 0);
+   if (scanrelid > 0)
+   estate->es_epqScanDone[scanrelid - 1] = false;
+   else
+   {
+   Bitmapset  *relids;
+   int rtindex = -1;
+
+   if (IsA(node->ps.plan, ForeignScan))
+   relids = ((ForeignScan *) 
node->ps.plan)->fs_relids;
+   else if (IsA(node->ps.plan, CustomScan))
+   relids = ((CustomScan *) 
node->ps.plan)->custom_relids;
+   else
+   elog(ERROR, "unexpected scan node: %d",
+(int)nodeTag(node->ps.plan));

-   estate->es_epqScanDone[scanrelid - 1] = false;
+   while ((rtindex = bms_next_member(relids, rtindex)) >= 
0)
+   {
+   Assert(rtindex > 0);
+   estate->es_epqScanDone[rtindex - 1] = false;
+   }
+   }
}

That seems the outerplan's business to me, so I think it'd be better to 
just return, right before the assertion, as I said before.  Seen from 
another angle, ISTM that FDWs that don't use a local join execution plan 
wouldn't need to be aware of handling the es_epqScanDone flags.  (Do you 
think that such FDWs should do something like what 

Re: [HACKERS] pg_stat_replication log positions vs base backups

[Magnus Hagander]

On Thu, Nov 26, 2015 at 1:03 PM, Michael Paquier 
wrote:

> On Thu, Nov 26, 2015 at 6:45 PM, Magnus Hagander wrote:
> > I'm only talking about the actual value in pg_stat_replication here, not
> > what we are using internally. These are two different things of course -
> > let's keep them separate for now. In pg_stat_replication, we explicitly
> > check for InvalidXLogRecPtr and then explicitly set the resulting value
> to
> > NULL in the SQL return.
>
> No objections from here. I guess you already have a patch?
>

Well, no, because I haven't figured out which way is the logical one - make
them all return NULL or make them all return 0/0...


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

Re: [HACKERS] WIP: About CMake v2

[Euler Taveira]

On 26-11-2015 07:33, YUriy Zhuravlev wrote:
> On Thursday 26 November 2015 01:29:37 Euler Taveira wrote:
>> I give it a try. Nice WIP. IMHO you should try to support cmake version
>> that are available in the stable releases. Looking at [1], I think the
>> best choice is 2.8.11 (because it will cover Red Hat based distros and
>> also Debian based ones). Are you using a new feature from 3.1? I mean,
>> it should be nice to cover old stable releases, if it is possible.
> Maybe you are right. But by the time I finish my work I think 3.0 will become 
> a standard. CMake is developing rapidly and soon will have version 3.4.1
> And one more thing: a normal documentation came with 3.0. :)
> But I try to check my code for 2.8.11, now I have 3.4.0 (latest for Gentoo).
> 
Have in mind that stable distros have a long cycle and are not released
soon. If you are planning your cmake work for 9.6 or even 9.7, it is
prudent to suport Red Hat 7 or Debian 8 because it will be a pain in the
neck to install a new cmake version just to compile postgres.


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


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

Re: [HACKERS] Foreign join pushdown vs EvalPlanQual

[Kouhei Kaigai]

> On 2015/11/26 14:04, Kouhei Kaigai wrote:
> >> On 2015/11/24 2:41, Robert Haas wrote:
> >>> On Fri, Nov 20, 2015 at 12:11 AM, Kouhei Kaigai  
> >>> wrote:
>  One subplan means FDW driver run an entire join sub-tree with local
>  alternative sub-plan; that is my expectation for the majority case.
> 
> >>> What I'm imagining is that we'd add handling that allows the
> >>> ForeignScan to have inner and outer children.  If the FDW wants to
> >>> delegate the EvalPlanQual handling to a local plan, it can use the
> >>> outer child for that.  Or the inner one, if it likes.  The other one
> >>> is available for some other purposes which we can't imagine yet.  If
> >>> this is too weird, we can only add handling for an outer subplan and
> >>> forget about having an inner subplan for now.  I just thought to make
> >>> it symmetric, since outer and inner subplans are pretty deeply baked
> >>> into the structure of the system.
> 
> >> I'd vote for only allowing an outer subplan.
> 
> > The attached patch adds: Path *fdw_outerpath field to ForeignPath node.
> > FDW driver can set arbitrary but one path-node here.
> > After that, this path-node shall be transformed to plan-node by
> > createplan.c, then passed to FDW driver using GetForeignPlan callback.
> 
> I understand this, as I also did the same thing in my patches, but
> actually, that seems a bit complicated to me.  Instead, could we keep
> the fdw_outerpath in the fdw_private of a ForeignPath node when creating
> the path node during GetForeignPaths, and then create an outerplan
> accordingly from the fdw_outerpath stored into the fdw_private during
> GetForeignPlan, by using create_plan_recurse there?  I think that that
> would make the core involvment much simpler.
>
How to use create_plan_recurse by extension? It is a static function.

> > We expect FDW driver set this plan-node on lefttree (a.k.a outerPlan).
> > The Plan->outerPlan is a common field, so patch size become relatively
> > small. FDW driver can initialize this plan at BeginForeignScan, then
> > execute this sub-plan-tree on demand.
> 
> Another idea would be to add the core support for
> initializing/closing/rescanning the outerplan tree when the tree is given.
>
No. Please don't repeat same discussion again.

> > Remaining portions are as previous version. ExecScanFetch is revised
> > to call recheckMtd always when scanrelid==0, then FDW driver can get
> > control using RecheckForeignScan callback.
> > It allows FDW driver to handle (1) EPQ recheck on underlying scan nodes,
> > (2) reconstruction of joined tuple, and (3) EPQ recheck on join clauses,
> > by its preferable implementation - including execution of an alternative
> > sub-plan.
> 
> @@ -85,6 +86,18 @@ ForeignRecheck(ForeignScanState *node, TupleTableSlot
> *slot)
> 
>   ResetExprContext(econtext);
> 
> + /*
> +  * FDW driver has to recheck visibility of EPQ tuple towards
> +  * the scan qualifiers once it gets pushed down.
> +  * In addition, if this node represents a join sub-tree, not
> +  * a scan, FDW driver is also responsible to reconstruct
> +  * a joined tuple according to the primitive EPQ tuples.
> +  */
> + if (fdwroutine->RecheckForeignScan)
> + {
> + if (!fdwroutine->RecheckForeignScan(node, slot))
> + return false;
> + }
> 
> Maybe I'm missing something, but I think we should let FDW do the work
> if scanrelid==0, not just if fdwroutine->RecheckForeignScan is given.
> (And if scanrelid==0 and fdwroutine->RecheckForeignScan is not given, we
> should abort the transaction.)
>
It should be Assert(). The node with scanrelid==0 never happen
unless FDW driver does not add such a path explicitly.

> Another thing I'm concerned about is
> 
> @@ -347,8 +355,26 @@ ExecScanReScan(ScanState *node)
>   {
>   Index   scanrelid = ((Scan *)
> node->ps.plan)->scanrelid;
> 
> - Assert(scanrelid > 0);
> + if (scanrelid > 0)
> + estate->es_epqScanDone[scanrelid - 1] = false;
> + else
> + {
> + Bitmapset  *relids;
> + int rtindex = -1;
> +
> + if (IsA(node->ps.plan, ForeignScan))
> + relids = ((ForeignScan *)
> node->ps.plan)->fs_relids;
> + else if (IsA(node->ps.plan, CustomScan))
> + relids = ((CustomScan *)
> node->ps.plan)->custom_relids;
> + else
> + elog(ERROR, "unexpected scan node: %d",
> +  (int)nodeTag(node->ps.plan));
> 
> - estate->es_epqScanDone[scanrelid - 1] = false;
> + while ((rtindex = bms_next_member(relids, rtindex)) >=
> 0)
> + {
> + Assert(rtindex > 0);
> + 

Re: [HACKERS] Foreign join pushdown vs EvalPlanQual

[Kouhei Kaigai]

> At Thu, 26 Nov 2015 05:04:32 +, Kouhei Kaigai  wrote
> in <9a28c8860f777e439aa12e8aea7694f801176...@bpxm15gp.gisp.nec.co.jp>
> > > On 2015/11/24 2:41, Robert Haas wrote:
> > > > On Fri, Nov 20, 2015 at 12:11 AM, Kouhei Kaigai  
> > > > wrote:
> > > >> One subplan means FDW driver run an entire join sub-tree with local
> > > >> alternative sub-plan; that is my expectation for the majority case.
> > >
> > > > What I'm imagining is that we'd add handling that allows the
> > > > ForeignScan to have inner and outer children.  If the FDW wants to
> > > > delegate the EvalPlanQual handling to a local plan, it can use the
> > > > outer child for that.  Or the inner one, if it likes.  The other one
> > > > is available for some other purposes which we can't imagine yet.  If
> > > > this is too weird, we can only add handling for an outer subplan and
> > > > forget about having an inner subplan for now.  I just thought to make
> > > > it symmetric, since outer and inner subplans are pretty deeply baked
> > > > into the structure of the system.
> > >
> > > I'd vote for only allowing an outer subplan.
> > >
> > The attached patch adds: Path *fdw_outerpath field to ForeignPath node.
> > FDW driver can set arbitrary but one path-node here.
> 
> It is named "outerpath/plan". Surely we used the term 'outer' in
> association with other nodes for disign decision but is it valid
> to call it outer?  Addition to that, there's no innerpath in this
> patch and have "path" instead.
>
Just "path" is too simple, not to inform people the expected usage
of the node.
If we would assign another name, my preference is "fdw_subpath" or
"fdw_altpath".

> > After that, this path-node shall be transformed to plan-node by
> > createplan.c, then passed to FDW driver using GetForeignPlan callback.
> > We expect FDW driver set this plan-node on lefttree (a.k.a outerPlan).
> > The Plan->outerPlan is a common field, so patch size become relatively
> 
>  Plan->outerPlan => Plan->lefttree?
>
Yes, s/outerPlan/lefttree/g

> > small. FDW driver can initialize this plan at BeginForeignScan, then
> > execute this sub-plan-tree on demand.
> >
> > Remaining portions are as previous version. ExecScanFetch is revised
> > to call recheckMtd always when scanrelid==0, then FDW driver can get
> > control using RecheckForeignScan callback.
> 
> Perhaps we need a comment about foreignscan as a fake join for
> the case with scanrelid == 0 in ExecScanReScan.
>
Indeed,

> > It allows FDW driver to handle (1) EPQ recheck on underlying scan nodes,
> > (2) reconstruction of joined tuple, and (3) EPQ recheck on join clauses,
> > by its preferable implementation - including execution of an alternative
> > sub-plan.
> 
> In ForeignRecheck, ExecQual on fdw_recheck_quals is executed if
> RecheckForeignScan returns true, which I think the signal that
> the returned tuple matches the recheck qual. Whether do you think
> have the responsibility to check the reconstructed tuple when
> RecheckCoreignScan is registered, RecheckForeignScan or ExecQual?
>
Only RecheckForeignScan can reconstruct a joined tuple. On the other
hands, both of facility can recheck scan-qualifiers and join-clauses.
FDW author can choose its design according to his preference.
If fdw_recheck_quals==NIL, FDW can apply all the rechecks within
RecheckForeignScan callback.


> The documentation says as following so I think the former has.
> 
> # I don't understhad what 'can or must' means, though... 'can and
> # must'?
> 
> + Also, this callback can or must recheck scan qualifiers and join
> + conditions which are pushed down. Especially, it needs special
>
If fdw_recheck_quals is set up correctly and join type is inner join,
FDW driver does not recheck by itself. Elsewhere, it has to recheck
the joined tuple, not only reconstruction.
I try to revise the SGML stuff.

> 
> > > There seems to be no changes to make_foreignscan.  Is that OK?
> > >
> > create_foreignscan_path(), not only make_foreignscan().
> >
> > This patch is not tested by actual FDW extensions, so it is helpful
> > to enhance postgres_fdw to run the alternative sub-plan on EPQ recheck.
> 
> regardes,
>
--
NEC Business Creation Division / PG-Strom Project
KaiGai Kohei 



-- 
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] Redefine default result from PQhost()?

[Tom Lane]

Magnus Hagander  writes:
> On Wed, Nov 25, 2015 at 11:59 PM, Tom Lane  wrote:
>> I think we should [ return DEFAULT_PGSOCKET_DIR not NULL ]

> I agree with this change in genera. But I wonder if there's a risk here
> that we break some applications isnt' it? It's clearly a backwards
> incompatible change, so wouldn't it require a bump of libpq version?

I don't see why it would need a libpq version bump, as it's not an ABI
breakage.  As for breaking application logic, it's true that any app code
that is explicitly checking for NULL would become dead code, but that
seems pretty harmless.  It's already the case that apps should be prepared
to get back an explicit socket directory path spec; that would merely
become a more common case than before.

Also, given the precedent of yesterday's psql fix, we should not discount
the idea that we will be *removing* not adding corner-case bugs in some
applications.  Failure to check for NULL is probably even more common in
other apps than in psql.

On the other side of the coin, it's worth noting that there is no
well-defined way for libpq-using apps to discover the value of
DEFAULT_PGSOCKET_DIR.  (psql is getting it by #include'ing an internal
header file; there is no way to get it from PQconndefaults.)  So whatever
an app might have been doing if it did check for NULL is arguably
inherently buggy, and bypassing such code will be a good thing.

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] WIP: About CMake v2

[Tom Lane]

Euler Taveira  writes:
> On 26-11-2015 07:33, YUriy Zhuravlev wrote:
>> Maybe you are right. But by the time I finish my work I think 3.0 will 
>> become 
>> a standard. CMake is developing rapidly and soon will have version 3.4.1
>> And one more thing: a normal documentation came with 3.0. :)
>> But I try to check my code for 2.8.11, now I have 3.4.0 (latest for Gentoo).

> Have in mind that stable distros have a long cycle and are not released
> soon. If you are planning your cmake work for 9.6 or even 9.7, it is
> prudent to suport Red Hat 7 or Debian 8 because it will be a pain in the
> neck to install a new cmake version just to compile postgres.

Not working with the cmake version shipped in current distributions would
almost certainly cause us to reject this patch.  Adding a new build
dependency is bad enough; adding one that isn't easily available is a
show-stopper.  You'd better think in terms of what's provided with RHEL6,
not RHEL7, as the minimum baseline on the Red Hat side.  I'm not sure what
the oldest active LTS distribution is in the Debian world, but I'm pretty
sure it won't have cmake 3.

(FWIW, RHEL6 seems to be carrying 2.8.12 currently.)

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] What .gitignore files do in the tarball?

[Robert Haas]

On Wed, Nov 25, 2015 at 11:49 PM, Tom Lane  wrote:
> Euler Taveira  writes:
>> +1 to remove all of those files.
>
> Meh.  We've always shipped that stuff; before git, we shipped .cvsignore
> files, and there were no complaints about it, going back twenty years at
> this point.  If the files amounted to anything meaningful space-wise,
> I would agree, but as things stand I see no value in removing them.
>
> One plausible argument for them being useful to downstream users is that
> they provide positive documentation as to what derived files can be
> expected to appear while building the code.  (In this connection, I note
> that CVS didn't produce complaints about stray files, so that we had to
> work quite a bit on the ignore-files when we converted from CVS to git.
> That seems like useful value-added information.)
>
> I also have a personal reason for not removing them, which is that
> I usually verify built tarballs by diff'ing them against my local git
> checkout.  I do not need the noise of a bunch of "Only in ..." complaints
> from that.

Yeah, agreed.  If somebody gets them and doesn't want them, it's a
one-line find command to nuke them all.

find . -name .gitignore -exec rm {} \;

On the other hand, if we remove them, putting them back is not quite so trivial.

-- 
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: About CMake v2

[YUriy Zhuravlev]

On Thursday 26 November 2015 11:10:36 you wrote:
> Adding a new build
> dependency is bad enough; adding one that isn't easily available is a
> show-stopper.

If someone decided to compile from source Postgres rather than install from 
RPM then it will not be a problem as to build CMake.
On the one hand it is good that the GNU Make a stable and have not changed, 
but it can not last forever.

It is possible to make support for CMake 2.6 but now is not the time when you 
have to think about it. I'll come back to this when all the functionality will 
work.

Thanks. 
-- 
YUriy Zhuravlev
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] WIP: About CMake v2

[Andres Freund]

On 2015-11-26 19:22:23 +0300, YUriy Zhuravlev wrote:
> On Thursday 26 November 2015 11:10:36 you wrote:
> > Adding a new build
> > dependency is bad enough; adding one that isn't easily available is a
> > show-stopper.
> 
> If someone decided to compile from source Postgres rather than install from 
> RPM then it will not be a problem as to build CMake.

Packages have to be built from something, and usually it's desirable
that all dependencies are from within the distribution.

I don't think you'll have much success pushing back on this.

> It is possible to make support for CMake 2.6 but now is not the time when you 
> have to think about it. I'll come back to this when all the functionality 
> will 
> work.

No point in doing any work if you don't agree with the basic prerequisites.


-- 
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: About CMake v2

[Tom Lane]

Andres Freund  writes:
> On 2015-11-26 19:22:23 +0300, YUriy Zhuravlev wrote:
>> On Thursday 26 November 2015 11:10:36 you wrote:
>>> Adding a new build dependency is bad enough; adding one that isn't
>>> easily available is a show-stopper.

>> If someone decided to compile from source Postgres rather than install from 
>> RPM then it will not be a problem as to build CMake.

> Packages have to be built from something, and usually it's desirable
> that all dependencies are from within the distribution.

Yeah.  In particular, RPMs don't magically appear out of nowhere; somebody
has to build them, and more often than not, the somebody is subject to
distro packaging policy that says what build dependencies are allowed.

Red Hat certainly isn't ever going to ship a package that requires some
version of cmake that isn't what they ship in the particular distribution,
for example.

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] WIP: About CMake v2

[YUriy Zhuravlev]

On Thursday 26 November 2015 17:42:16 you wrote:
> No point in doing any work if you don't agree with the basic prerequisites.
I meant that support for older versions of CMake I'll do when will implement 
other functions.
-- 
YUriy Zhuravlev
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] WIP: About CMake v2

[Alvaro Herrera]

Tom Lane wrote:

> Not working with the cmake version shipped in current distributions would
> almost certainly cause us to reject this patch.  Adding a new build
> dependency is bad enough; adding one that isn't easily available is a
> show-stopper.  You'd better think in terms of what's provided with RHEL6,
> not RHEL7, as the minimum baseline on the Red Hat side.  I'm not sure what
> the oldest active LTS distribution is in the Debian world, but I'm pretty
> sure it won't have cmake 3.

Current Debian Stable (wheezy) has cmake 3.0.2.  Oldstable (jessie) has
2.8.4.

As for Ubuntu, according to https://wiki.ubuntu.com/LTS the oldest still
supported is 12.04 (Precise Pangolin) and they have 2.8.7:
http://packages.ubuntu.com/precise/cmake

For these systems, the CMake 2.8 functionality would be enough.  No need
to go back to 2.6.

-- 
Á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] New email address

[Stefan Kaltenbrunner]

On 11/24/2015 11:03 PM, José Luis Tallón wrote:
> On 11/24/2015 07:55 PM, Tom Lane wrote:
>> [snip]
>> The clearly critical thing, though, is that when forwarding a message
>> from
>> a person at a DMARC-using domain, we would have to replace the From: line
>> with something @postgresql.org.  This is what gets it out from under the
>> original domain's DMARC policy.
> 
> One possibility that comes to mind:
> 
> - Remove the sender's DMARC headers+signature **after thoroughly
> checking it** (to minimize the amount of UBE/UCE/junk going in)
> - Replace the sender's (i.e. 'From:' header) with
> list-sender+munched-em...@postgresql.org (VERP-ified address)
> 
> - Add the required headers, footers, change the subject line, etc
> 
> - DKIM-sign the resulting message with postgresql.org's keys before
> sending it

that seems entirely doable with our current infrastructure (and even
with minimal-to-no hackery on mj2) - but it still carries the "changes
From:" issue :/


>> [snip]
>>
>> If Rudy's right that Gmail is likely to start using p=reject DMARC
>> policy,
>> we are going to have to do something about this before that; we have too
>> many people on gmail.  I'm not exactly in love with replacing From:
>> headers but there may be little alternative.  We could do something like
>> From: Persons Real Name 
>> Reply-To: ...
>> so that at least the person's name would still be readable in MUA
>> displays.
> Yup
> 
>> We'd have to figure out whether we want the Reply-To: to be the original
>> author or the list; as I recall, neither of those are fully satisfactory.
> Or just strip it, though that trump the sender's explicit preference
> (expressed by setting the header)
> 
> 
> I might be able to help a bit with implementation if needed.

the MTA side of things is fairly easy/straightforward(including using
exim for some of the heavy lifting like doing the inbound dkim checking
and "hinting" the downstream ML-boxes with the results) - however the
main mailinglist infrastructure is still mj2 and that is aeons old perl
- still interested in helping with implementation? ;)


Stefan


-- 
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 Tue, Nov 17, 2015 at 6:30 PM, Simon Riggs  wrote:

> On 17 November 2015 at 11:48, Amit Kapila  wrote:
>
>>
>> I think in that case what we can do is if the total number of
>> sub transactions is lesser than equal to 64 (we can find that by
>> overflowed flag in PGXact) , then apply this optimisation, else use
>> the existing flow to update the transaction status.  I think for that we
>> don't even need to reserve any additional memory. Does that sound
>> sensible to you?
>>
>
> I understand you to mean that the leader should look backwards through the
> queue collecting xids while !(PGXACT->overflowed)
>
> No additional shmem is required
>
>
Okay, as discussed I have handled the case of sub-transactions without
additional shmem in the attached patch.  Apart from that, I have tried
to apply this optimization for Prepared transactions as well, but as
the dummy proc used for such transactions doesn't have semaphore like
backend proc's, so it is not possible to use such a proc in group status
updation as each group member needs to wait on semaphore.  It is not tad
difficult to add the support for that case if we are okay with creating
additional
semaphore for each such dummy proc which I was not sure, so I have left
it for now.


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


group_update_clog_v2.patch
Description: Binary data

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