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

[Pavel Stehule]

2016-04-02 7:16 GMT+02:00 Pavel Stehule :

> Hi
>
> 2016-04-01 10:21 GMT+02:00 Kyotaro HORIGUCHI <
> horiguchi.kyot...@lab.ntt.co.jp>:
>
>> Hello, sorry for being a bit late.
>> The attatched are the new version of the patch.. set.
>>
>> 1. 0001-Suggest-IF-NOT-EXISTS-for-tab-completion-of-psql.patch
>>
>>  Adds IF (NOT) EXISTS completion. It doesn't fix the issue that
>>  the case of additional keywords don't follow the input.
>>
>> 2. 0002-Make-added-keywords-for-completion-queries-follow-to.patch
>>
>>  Fixes the case-don't-follow issue by introducing a new macro set
>>  ADDLISTn(). This leaves the issue for keywords along with
>>  attributes.
>>
>> 3. 0003-Make-COMPLETE_WITH_ATTR-to-accept-additional-keyword.patch
>>
>>   Fixes the issue left after 0002 patch.
>>   This patch does the following
>>   things.
>>
>>   1. Change completion_charp from const char * to PQExpBuffer.
>>
>>   2. Chnage COMPLETE_WITH_QUERY and COMPLETE_WITH_ATTR to accept
>>  an expression instead of string literal.
>>
>>   3. Replace all additional keyword lists in psql_copmletion with
>>  ADDLISTn() expression.
>>
>>
>> At Fri, 01 Apr 2016 11:52:03 +0900 (Tokyo Standard Time), Kyotaro
>> HORIGUCHI  wrote in <
>> 20160401.115203.98896697.horiguchi.kyot...@lab.ntt.co.jp>
>> > > > > I found new warning
>> > > > >
>> > > > >  tab-complete.c:1438:87: warning: right-hand operand of comma
>> expression
>> > > > > has no effect [-Wunused-value]
>> > > >
>> > > > Mmm. Google said me that gcc 4.9 does so. I'm using 4.8.5 so I
>> > > > haven't see the warning.
>> > > >
>> > > > https://gcc.gnu.org/gcc-4.9/porting_to.html
>> > > >
>> > > > 1436:   else if (HeadMatches2("CREATE", "SCHEMA") &&
>> > > > 1437:SHIFT_TO_LAST1("CREATE") &&
>> > > > 1438:false) {} /* FALL THROUGH */
>> ...
>> > > > But the right hand value (true) is actually "used" in the
>> > > > expression (even though not effective). Perhaps (true && false)
>> > > > was potimized as false and the true is regarded to be unused?
>> > > > That's stupid.. Using functions instead of macros seems to solve
>> > > > this but they needed to be wraped by macros as
>> > > > additional_kw_query(). That's a pain..
>> ...
>> > This needs to use gcc 4.9 to address, but CentOS7 doesn't have
>> > devtools-2 repo so now I'm building CentOS6 environment for this
>> > purpose. Please wait for a while.
>>
>> Finally I settled it by replacing comma expression with logical
>> OR or AND expresssion. gcc 4.9 compains for some unused variables
>> in flex output but it is the another issue.
>>
>> I forgot to address COMPLETE_WITH_ATTTR but it needed an overhaul
>> of some macros and changing the type of completion_charp. The
>> third patch does it but it might be unacceptable..
>>
>>
> something is wrong, autocomplete for CREATE TABLE IF NOT EXISTS doesn't
> work
>

CREATE UNLOGGED/TEMP table is working.

Pavel


>
> Regards
>
> Pavel
>
>
>> regards,
>>
>> --
>> Kyotaro Horiguchi
>> NTT Open Source Software Center
>>
>
>

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

[Pavel Stehule]

Hi

2016-04-01 10:21 GMT+02:00 Kyotaro HORIGUCHI <
horiguchi.kyot...@lab.ntt.co.jp>:

> Hello, sorry for being a bit late.
> The attatched are the new version of the patch.. set.
>
> 1. 0001-Suggest-IF-NOT-EXISTS-for-tab-completion-of-psql.patch
>
>  Adds IF (NOT) EXISTS completion. It doesn't fix the issue that
>  the case of additional keywords don't follow the input.
>
> 2. 0002-Make-added-keywords-for-completion-queries-follow-to.patch
>
>  Fixes the case-don't-follow issue by introducing a new macro set
>  ADDLISTn(). This leaves the issue for keywords along with
>  attributes.
>
> 3. 0003-Make-COMPLETE_WITH_ATTR-to-accept-additional-keyword.patch
>
>   Fixes the issue left after 0002 patch.
>   This patch does the following
>   things.
>
>   1. Change completion_charp from const char * to PQExpBuffer.
>
>   2. Chnage COMPLETE_WITH_QUERY and COMPLETE_WITH_ATTR to accept
>  an expression instead of string literal.
>
>   3. Replace all additional keyword lists in psql_copmletion with
>  ADDLISTn() expression.
>
>
> At Fri, 01 Apr 2016 11:52:03 +0900 (Tokyo Standard Time), Kyotaro
> HORIGUCHI  wrote in <
> 20160401.115203.98896697.horiguchi.kyot...@lab.ntt.co.jp>
> > > > > I found new warning
> > > > >
> > > > >  tab-complete.c:1438:87: warning: right-hand operand of comma
> expression
> > > > > has no effect [-Wunused-value]
> > > >
> > > > Mmm. Google said me that gcc 4.9 does so. I'm using 4.8.5 so I
> > > > haven't see the warning.
> > > >
> > > > https://gcc.gnu.org/gcc-4.9/porting_to.html
> > > >
> > > > 1436:   else if (HeadMatches2("CREATE", "SCHEMA") &&
> > > > 1437:SHIFT_TO_LAST1("CREATE") &&
> > > > 1438:false) {} /* FALL THROUGH */
> ...
> > > > But the right hand value (true) is actually "used" in the
> > > > expression (even though not effective). Perhaps (true && false)
> > > > was potimized as false and the true is regarded to be unused?
> > > > That's stupid.. Using functions instead of macros seems to solve
> > > > this but they needed to be wraped by macros as
> > > > additional_kw_query(). That's a pain..
> ...
> > This needs to use gcc 4.9 to address, but CentOS7 doesn't have
> > devtools-2 repo so now I'm building CentOS6 environment for this
> > purpose. Please wait for a while.
>
> Finally I settled it by replacing comma expression with logical
> OR or AND expresssion. gcc 4.9 compains for some unused variables
> in flex output but it is the another issue.
>
> I forgot to address COMPLETE_WITH_ATTTR but it needed an overhaul
> of some macros and changing the type of completion_charp. The
> third patch does it but it might be unacceptable..
>
>
something is wrong, autocomplete for CREATE TABLE IF NOT EXISTS doesn't work

Regards

Pavel


> regards,
>
> --
> Kyotaro Horiguchi
> NTT Open Source Software Center
>

Re: [HACKERS] Re: [COMMITTERS] pgsql: Refer to a TOKEN_USER payload as a "token user, " not as a "user

[Tom Lane]

Stephen Frost  writes:
> * Noah Misch (n...@leadboat.com) wrote:
>> I see some advantages of writing "TokenUser", as you propose.  However, our
>> error style guide says "Avoid mentioning called function names, either;
>> instead say what the code was trying to do."  Mentioning an enumerator name 
>> is
>> morally similar to mentioning a function name.

> That's a fair point, but it doesn't mean we should use a different
> spelling for the enumerator name to avoid that piece of the policy.  I
> certianly don't see "token user" as saying "what the code was trying to
> do" in this case.

FWIW, "token user" conveys entirely inappropriate, politically incorrect
connotations to me ;-).  I don't have any great suggestions on what to use
instead, but I share Stephen's unhappiness with the wording as-committed.

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] Re: [COMMITTERS] pgsql: Refer to a TOKEN_USER payload as a "token user, " not as a "user

[Stephen Frost]

* Noah Misch (n...@leadboat.com) wrote:
> On Fri, Apr 01, 2016 at 10:12:12PM -0400, Stephen Frost wrote:
> > and there's no such thing as a "token user" concept.  There's an enum,
> > one value of which is "TokenUser" and that's what we're asking the OS to
> > provide us info about, but I'd argue that if we're going to refer to the
> > textual enum representation then we should spell it just exactly as the
> > enum has it.
> > 
> > If we don't want to use "TokenUser" then I'd suggest that "user token"
> > is a more accurate term to use, as we had before this change.  There is
> > no such thing as a "token user", as far as I'm aware, in GSSAPI, SSPI,
> > or general access token lingo.
> 
> "User token" has definitely been wrong.  We already possess the user token at
> the moments of these error messages, because we pass the user token as the
> first GetTokenInformation() argument.  We're retrieving information about the
> "user" that pertains to a particular "token", hence "token user."  A verbose
> description is "could not get user associated with access token."

Ok, "user token information" would still be better than "token user"
which has a completely different connotation, as I see it.

> I see some advantages of writing "TokenUser", as you propose.  However, our
> error style guide says "Avoid mentioning called function names, either;
> instead say what the code was trying to do."  Mentioning an enumerator name is
> morally similar to mentioning a function name.

That's a fair point, but it doesn't mean we should use a different
spelling for the enumerator name to avoid that piece of the policy.  I
certianly don't see "token user" as saying "what the code was trying to
do" in this case.

Thanks!

Stephen


signature.asc
Description: Digital signature

[HACKERS] Re: [COMMITTERS] pgsql: Refer to a TOKEN_USER payload as a "token user, " not as a "user

[Noah Misch]

On Fri, Apr 01, 2016 at 10:12:12PM -0400, Stephen Frost wrote:
> * Noah Misch (n...@leadboat.com) wrote:
> > Refer to a TOKEN_USER payload as a "token user," not as a "user token".
> > 
> > This corrects messages for can't-happen errors.  The corresponding "user
> > token" appears in the HANDLE argument of GetTokenInformation().
> 
> I'm not at all convinced that this is an improvement.  I understand that
> it's a "can't happen" case, but we're calling out to a OS function and
> as much as things "can't happen" they do, in fact, occationally happen,

They do, yes.  I mentioned that for the purpose of hinting that this commit
does not warrant release notes coverage.

> and there's no such thing as a "token user" concept.  There's an enum,
> one value of which is "TokenUser" and that's what we're asking the OS to
> provide us info about, but I'd argue that if we're going to refer to the
> textual enum representation then we should spell it just exactly as the
> enum has it.
> 
> If we don't want to use "TokenUser" then I'd suggest that "user token"
> is a more accurate term to use, as we had before this change.  There is
> no such thing as a "token user", as far as I'm aware, in GSSAPI, SSPI,
> or general access token lingo.

"User token" has definitely been wrong.  We already possess the user token at
the moments of these error messages, because we pass the user token as the
first GetTokenInformation() argument.  We're retrieving information about the
"user" that pertains to a particular "token", hence "token user."  A verbose
description is "could not get user associated with access token."

I see some advantages of writing "TokenUser", as you propose.  However, our
error style guide says "Avoid mentioning called function names, either;
instead say what the code was trying to do."  Mentioning an enumerator name is
morally similar to mentioning a function name.


-- 
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: [COMMITTERS] pgsql: Refer to a TOKEN_USER payload as a "token user, " not as a "user

[Stephen Frost]

* Noah Misch (n...@leadboat.com) wrote:
> Refer to a TOKEN_USER payload as a "token user," not as a "user token".
> 
> This corrects messages for can't-happen errors.  The corresponding "user
> token" appears in the HANDLE argument of GetTokenInformation().

I'm not at all convinced that this is an improvement.  I understand that
it's a "can't happen" case, but we're calling out to a OS function and
as much as things "can't happen" they do, in fact, occationally happen,
and there's no such thing as a "token user" concept.  There's an enum,
one value of which is "TokenUser" and that's what we're asking the OS to
provide us info about, but I'd argue that if we're going to refer to the
textual enum representation then we should spell it just exactly as the
enum has it.

If we don't want to use "TokenUser" then I'd suggest that "user token"
is a more accurate term to use, as we had before this change.  There is
no such thing as a "token user", as far as I'm aware, in GSSAPI, SSPI,
or general access token lingo.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] fd.c: flush data problems on osx

[Noah Misch]

On Mon, Mar 21, 2016 at 03:09:56PM +0300, Stas Kelvich wrote:
> On 21 Mar 2016, at 14:53, Andres Freund  wrote:
> > Hm. I think we should rather just skip calling pg_flush_data in the
> > directory case, that very likely isn't beneficial on any OS.
> 
> Seems reasonable, changed.
> 
> > I think we still need to fix the mmap() implementation to support the
> > offset = 0, nbytes = 0 case (via fseek(SEEK_END).
> 
> It is already in this diff. I’ve added this few messages ago.

[This is a generic notification.]

The above-described topic is currently a PostgreSQL 9.6 open item.  Andres,
since you committed the patch believed to have created it, you own this open
item.  If that responsibility lies elsewhere, please let us know whose
responsibility it is to fix this.  Since new open items may be discovered at
any time and I want to plan to have them all fixed well in advance of the ship
date, I will appreciate your efforts toward speedy resolution.  Please
present, within 72 hours, a plan to fix the defect within seven days of this
message.  Thanks.


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

[Thomas Munro]

On Thu, Mar 31, 2016 at 5:11 PM, Thomas Munro
 wrote:
> On Thu, Mar 31, 2016 at 3:55 AM, Masahiko Sawada  
> wrote:
>> On Wed, Mar 30, 2016 at 11:43 PM, Masahiko Sawada  
>> wrote:
>>> On Tue, Mar 29, 2016 at 5:36 PM, Kyotaro HORIGUCHI
>>>  wrote:
 I personally don't think it needs such a survive measure. It is
 very small syntax and the parser reads very short text. If the
 parser failes in such mode, something more serious should have
 occurred.

 At Tue, 29 Mar 2016 16:51:02 +0900, Fujii Masao  
 wrote in 
 
> On Tue, Mar 29, 2016 at 4:23 PM, Kyotaro HORIGUCHI
>  wrote:
> > Hello,
> >
> > At Mon, 28 Mar 2016 18:38:22 +0900, Masahiko Sawada 
> >  wrote in 
> > 
> > sawada.mshk> On Mon, Mar 28, 2016 at 5:50 PM, Kyotaro HORIGUCHI
> >>  wrote:
> > As mentioned in my comment, SQL parser converts yy_fatal_error
> > into ereport(ERROR), which can be caught by the upper PG_TRY (by
> > #define'ing fprintf). So it is doable if you mind exit().
>
> I'm afraid that your idea doesn't work in postmaster. Because 
> ereport(ERROR) is
> implicitly promoted to ereport(FATAL) in postmaster. IOW, when an internal
> flex fatal error occurs, postmaster just exits instead of jumping out of 
> parser.

 If The ERROR may be LOG or DEBUG2 either, if we think the parser
 fatal erros are recoverable. guc-file.l is doing so.

> ISTM that, when an internal flex fatal error occurs, it's
> better to elog(FATAL) and terminate the problematic
> process. This might lead to the server crash (e.g., if
> postmaster emits a FATAL error, it and its all child processes
> will exit soon). But probably we can live with this because the
> fatal error basically rarely happens.

 I agree to this

> OTOH, if we make the process keep running even after it gets an internal
> fatal error (like Sawada's patch or your idea do), this might cause more
> serious problem. Please imagine the case where one walsender gets the 
> fatal
> error (e.g., because of OOM), abandon new setting value of
> synchronous_standby_names, and keep running with the previous setting 
> value.
> OTOH, the other walsender processes successfully parse the setting and
> keep running with new setting. In this case, the inconsistency of the 
> setting
> which each walsender is based on happens. This completely will mess up the
> synchronous replication.

 On the other hand, guc-file.l seems ignoring parser errors under
 normal operation, even though it may cause similar inconsistency,
 if any..

 | LOG:  received SIGHUP, reloading configuration files
 | LOG:  input in flex scanner failed at file 
 "/home/horiguti/data/data_work/postgresql.conf" line 1
 | LOG:  configuration file "/home/horiguti/data/data_work/postgresql.conf" 
 contains errors; no changes were applied

> Therefore, I think that it's better to make the problematic process exit
> with FATAL error rather than ignore the error and keep it running.

 +1. Restarting walsender would be far less harmful than keeping
 it running in doubtful state.

 Sould I wait for the next version or have a look on the latest?

>>>
>>> Attached latest patch incorporate some review comments so far, and is
>>> rebased against current HEAD.
>>>
>>
>> Sorry I attached wrong patch.
>> Attached patch is correct patch.
>>
>> [mulit_sync_replication_v21.patch]
>
> Here are some TPS numbers from some quick tests I ran on a set of
> Amazon EC2 m3.large instances ("2 vCPU" virtual machines) configured
> as primary + 3 standbys, to try out different combinations of
> synchronous_commit levels and synchronous_standby_names numbers.  They
> were run for a short time only and these are of course systems with
> limited and perhaps uneven IO and CPU, but they still give some idea
> of the trends.  And reassuringly, the trends are travelling in the
> expected directions.
>
> All default settings except shared_buffers = 1GB, and the GUCs
> required for replication.
>
> pgbench postgres -j2 -c2 -N bench2 -T 600
>
>1(*) 2(*) 3(*)
>  
> off  = 4056 4096 4092
> local= 1323 1299 1312
> remote_write = 1130 1046  958
> on   =  860  744  701
> remote_apply =  785  725  604
>
> pgbench postgres -j16 -c16 -N bench2 -T 600
>
>1(*) 2(*) 3(*)
>  
> off  = 3952 3943 3933
> local= 2964 2984 3026
> remote_write = 2790 

Re: [HACKERS] Using quicksort for every external sort run

[Peter Geoghegan]

On Thu, Feb 4, 2016 at 3:14 AM, Peter Geoghegan  wrote:
> Nyberg et al may have said it best in 1994, in the Alphasort Paper [1]:

This paper is available from
http://www.vldb.org/journal/VLDBJ4/P603.pdf (previously link is now
dead)

> The paper also has very good analysis of the economics of sorting:
>
> "Even for surprisingly large sorts, it is economical to perform the
> sort in one pass."

I suggest taking a look at "Figure 2. Replacement-selection sort vs.
QuickSort" in the paper. It confirms what I said recently about cache
size. The diagram is annotated: "The tournament tree of
replacement-selection sort at left has bad cache behavior, unless the
entire tournament fits in cache". I think we're well justified in
giving no weight at all to cases where the *entire* tournament tree
(heap) fits in cache, because it's not economical to use a
cpu-cache-sized work_mem setting. It simply makes no sense.

I understand the reluctance to give up on replacement selection. The
authors of this paper were themselves reluctant to do so. As they put
it:

"""
We were reluctant to abandon replacement-selection sort, because it has
stability and it generates long runs. Our first approach was to improve
replacement-selection sort's cache locality. Standard replacement-selection
sort has terrible cache behavior, unless the tournament fits in cache. The
cache thrashes on the bottom levels of the tournament. If you think of the
tournament as a tree, each replacement-selection step traverses a path from a
pseudo-random leaf of the tree to the root. The upper parts of the tree may be
cache resident, but the bulk of the tree is not.

We investigated a replacement-selection sort that clusters tournament nodes so
that most parent-child node pairs are contained in the same cache line. This
technique reduces cache misses by a factor of two or three. Nevertheless,
replacement-selection sort is still less attractive than QuickSort because:

1. The cache behavior demonstrates less locality than QuickSorts. Even when
QuickSort runs did not fit entirely in cache, the average compare-exchange
time did not increase significantly.

2. Tournament sort is more CPU-intensive than QuickSort. Knuth calculated a 2:1
ratio for the programs he wrote. We observed a 2.5:1 speed advantage for
QuickSort over the best tournament sort we wrote.

The key to achieving high execution speeds on fast processors is to minimize
the number of references that cannot be serviced by the on-board cache (4MB in
the case of the DEC 7000 AXP). As mentioned before, QuickSort's memory access
patterns are sequential and, thus, have good cache behavior

"""

This paper is co-authored by Jim Gray, a Turing award laureate, as
well as some other very notable researchers. The paper appeared in
"Readings in Database Systems, 4th edition", which was edited by by
Joseph Hellerstein and Michael Stonebraker. These days, the cheapest
consumer level CPUs have 4MB caches (in 1994, that was exceptional),
so if this analysis wasn't totally justified in 1994, when the paper
was written, it is today.

I've spent a lot of time analyzing this problem. I've been looking at
external sorting in detail for almost a year now. I've done my best to
avoid any low-end regressions. I am very confident that I cannot do
any better than I already have there, though. If various very
influential figures in the database research community could not do
better, then I have doubts that we can. I started with the intuition
that we should still use replacement selection myself, but that just
isn't well supported by benchmarking cases with sensible
work_mem:cache size ratios.

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

[Michael Paquier]

On Fri, Apr 1, 2016 at 10:53 PM, Stas Kelvich  wrote:
> I wrote:
>> While testing the patch, I found a bug in the recovery conflict code
>> path. You can do the following to reproduce it:
>> 1) Start a master with a standby
>> 2) prepare a transaction on master
>> 3) Stop immediate on standby to force replay
>> 4) commit prepare transaction on master
>> 5) When starting the standby, it remains stuck here:
>
> Hm, I wasn’t able to reproduce that. Do you mean following scenario or am I 
> missing something?
>
> (async replication)
>
> $node_master->psql('postgres', "
> begin;
> insert into t values (1);
> prepare transaction 'x';
> ");
> $node_slave->teardown_node;
> $node_master->psql('postgres',"commit prepared 'x'");
> $node_slave->start;
> $node_slave->psql('postgres',"select count(*) from pg_prepared_xacts", stdout 
> => \$psql_out);
> is($psql_out, '0', "Commit prepared on master while slave is down.");

Actually, not exactly, the transaction prepared on master created a
table. Sorry for the lack of precisions in my review.
-- 
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] More stable query plans via more predictable column statistics

[Shulgin, Oleksandr]

On Apr 1, 2016 23:14, "Tom Lane"  wrote:
>
> "Shulgin, Oleksandr"  writes:
> > Alright.  I'm attaching the latest version of this patch split in two
> > parts: the first one is NULLs-related bugfix and the second is the
> > "improvement" part, which applies on top of the first one.
>
> I've applied the first of these patches,

Great news, thank you!

> broken into two parts first
> because it seemed like there were two issues and second because Tomas
> deserved primary credit for one part, ie realizing we were using the
> Haas-Stokes formula wrong.
>
> As for the other part, I committed it with one non-cosmetic change:
> I do not think it is right to omit "too wide" values when considering
> the threshold for MCVs.  As submitted, the patch was inconsistent on
> that point anyway since it did it differently in compute_distinct_stats
> and compute_scalar_stats.  But the larger picture here is that we define
> the MCV population to exclude nulls, so it's reasonable to consider a
> value as an MCV even if it's greatly outnumbered by nulls.  There is
> no such exclusion for "too wide" values; those things are just an
> implementation limitation in analyze.c, not something that is part of
> the pg_statistic definition.  If there are a lot of "too wide" values
> in the sample, we don't know whether any of them are duplicates, but
> we do know that the frequencies of the normal-width values have to be
> discounted appropriately.

Okay.

> Haven't looked at 0002 yet.

[crosses fingers] hope you'll have a chance to do that before feature
freeze for 9.6…

--
Alex

Re: [HACKERS] syntax sugar for conditional check

[David G. Johnston]

On Fri, Apr 1, 2016 at 3:22 PM, Jim Nasby  wrote:

> On 4/1/16 1:08 PM, Tom Lane wrote:
>
>> Jim Nasby  writes:
>>
>>> Rather than this, I think an exclusive-or operator would be a lot more
>>> useful. The only difficulty I run into with CHECK constaints is when I
>>> want to ensure that only ONE condition is true.
>>>
>>
>> "bool != bool" works as XOR.  If you need "exactly one of N" you could
>> do something like "(cond1::int + cond2::int + ...) = 1".  We could
>> wrap some syntactic sugar around either of these, but it's not clear
>> to me that it'd be any more useful than a custom SQL function.
>>
>
> It would prevent having to re-create that function every time... :)


​And it would nicely complement our recent addition of "
num_nonnulls
​(variadic "any")​"

David J.

Re: [HACKERS] [PATCH v11] GSSAPI encryption support

[Robbie Harwood]

Hello friends,

Song and dance, here's v11 both here and on my github:
https://github.com/frozencemetery/postgres/tree/feature/gssencrypt11

Changes from v10:

- Attempt to address a crash Michael is observing by switching to using
  the StringInfo/pqExpBuffer management functions over my own code as
  much as possible.  Michael, if this doesn't fix it, I'm out of ideas.
  Since I still can't reproduce this locally (left a client machine and
  a process on the same machine retrying for over an hour on your test
  case and didn't see it), could you provide me with some more
  information on why repalloc is complaining?  Is this a low memory
  situation where alloc might have failed?  What's your setup look like?
  That pointer looks like it's on the heap, is that correct?  I really
  don't have a handle on what's gone wrong here.

- Switch to using parse_bool for handling gss_encrypt.

- Remove accidental whitespace change.

Thanks!
From 945805d45e8021f92ad73518b3a74ac6bab89525 Mon Sep 17 00:00:00 2001
From: Robbie Harwood 
Date: Fri, 26 Feb 2016 16:07:05 -0500
Subject: [PATCH 1/3] Move common GSSAPI code into its own files

On both the frontend and backend, prepare for GSSAPI encryption suport
by moving common code for error handling into a common file.  Other than
build-system changes, no code changes occur in this patch.

Thanks to Michael Paquier for the Windows fixes.
---
 configure   |  2 +
 configure.in|  1 +
 src/Makefile.global.in  |  1 +
 src/backend/libpq/Makefile  |  4 ++
 src/backend/libpq/auth.c| 63 +---
 src/backend/libpq/be-gssapi-common.c| 74 +
 src/include/libpq/be-gssapi-common.h| 26 
 src/interfaces/libpq/Makefile   |  4 ++
 src/interfaces/libpq/fe-auth.c  | 48 +
 src/interfaces/libpq/fe-gssapi-common.c | 63 
 src/interfaces/libpq/fe-gssapi-common.h | 21 ++
 src/tools/msvc/Mkvcbuild.pm | 18 ++--
 12 files changed, 212 insertions(+), 113 deletions(-)
 create mode 100644 src/backend/libpq/be-gssapi-common.c
 create mode 100644 src/include/libpq/be-gssapi-common.h
 create mode 100644 src/interfaces/libpq/fe-gssapi-common.c
 create mode 100644 src/interfaces/libpq/fe-gssapi-common.h

diff --git a/configure b/configure
index b3f3abe..a5bd629 100755
--- a/configure
+++ b/configure
@@ -713,6 +713,7 @@ with_systemd
 with_selinux
 with_openssl
 krb_srvtab
+with_gssapi
 with_python
 with_perl
 with_tcl
@@ -5491,6 +5492,7 @@ $as_echo "$with_gssapi" >&6; }
 
 
 
+
 #
 # Kerberos configuration parameters
 #
diff --git a/configure.in b/configure.in
index 0bd90d7..4fd8f05 100644
--- a/configure.in
+++ b/configure.in
@@ -636,6 +636,7 @@ PGAC_ARG_BOOL(with, gssapi, no, [build with GSSAPI support],
   krb_srvtab="FILE:\$(sysconfdir)/krb5.keytab"
 ])
 AC_MSG_RESULT([$with_gssapi])
+AC_SUBST(with_gssapi)
 
 
 AC_SUBST(krb_srvtab)
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index e94d6a5..3dbc5c2 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -183,6 +183,7 @@ with_perl	= @with_perl@
 with_python	= @with_python@
 with_tcl	= @with_tcl@
 with_openssl	= @with_openssl@
+with_gssapi	= @with_gssapi@
 with_selinux	= @with_selinux@
 with_systemd	= @with_systemd@
 with_libxml	= @with_libxml@
diff --git a/src/backend/libpq/Makefile b/src/backend/libpq/Makefile
index 09410c4..a8cc9a0 100644
--- a/src/backend/libpq/Makefile
+++ b/src/backend/libpq/Makefile
@@ -21,4 +21,8 @@ ifeq ($(with_openssl),yes)
 OBJS += be-secure-openssl.o
 endif
 
+ifeq ($(with_gssapi),yes)
+OBJS += be-gssapi-common.o
+endif
+
 include $(top_srcdir)/src/backend/common.mk
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index 57c2f48..73d493e 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -136,11 +136,7 @@ bool		pg_krb_caseins_users;
  *
  */
 #ifdef ENABLE_GSS
-#if defined(HAVE_GSSAPI_H)
-#include 
-#else
-#include 
-#endif
+#include "libpq/be-gssapi-common.h"
 
 static int	pg_GSS_recvauth(Port *port);
 #endif   /* ENABLE_GSS */
@@ -715,63 +711,6 @@ recv_and_check_password_packet(Port *port, char **logdetail)
  */
 #ifdef ENABLE_GSS
 
-#if defined(WIN32) && !defined(WIN32_ONLY_COMPILER)
-/*
- * MIT Kerberos GSSAPI DLL doesn't properly export the symbols for MingW
- * that contain the OIDs required. Redefine here, values copied
- * from src/athena/auth/krb5/src/lib/gssapi/generic/gssapi_generic.c
- */
-static const gss_OID_desc GSS_C_NT_USER_NAME_desc =
-{10, (void *) "\x2a\x86\x48\x86\xf7\x12\x01\x02\x01\x02"};
-static GSS_DLLIMP gss_OID GSS_C_NT_USER_NAME = _C_NT_USER_NAME_desc;
-#endif
-
-
-static void
-pg_GSS_error(int severity, char *errmsg, OM_uint32 maj_stat, OM_uint32 min_stat)
-{
-	gss_buffer_desc gmsg;
-	OM_uint32	

Re: [HACKERS] syntax sugar for conditional check

[Jim Nasby]

On 4/1/16 1:08 PM, Tom Lane wrote:

Jim Nasby  writes:

Rather than this, I think an exclusive-or operator would be a lot more
useful. The only difficulty I run into with CHECK constaints is when I
want to ensure that only ONE condition is true.


"bool != bool" works as XOR.  If you need "exactly one of N" you could
do something like "(cond1::int + cond2::int + ...) = 1".  We could
wrap some syntactic sugar around either of these, but it's not clear
to me that it'd be any more useful than a custom SQL function.


It would prevent having to re-create that function every time... :)
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com


--
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_upgrade 9.6->9.6: column "amtype" does not exist

[Tom Lane]

Alvaro Herrera  writes:
> The reason for the failure is that pg_dump knows that 9.6 installations
> have the amtype column -- but on your older devel 9.6 install, it
> doesn't exist.  To fix it we would have to compare catalog versions in
> pg_dump rather than major version, but we don't that anywhere.  Anyway
> many upgrades could fail depending on other patches because of the same
> reason.  I don't think we need to fix this.

We've never expected pg_dump to be able to handle intermediate development
versions.  It's enough of a maintenance burden that it knows how to dump
from all previous stable branches.

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_upgrade 9.6->9.6: column "amtype" does not exist

[Alvaro Herrera]

Christoph Berg wrote:
> Hi,
> 
> I'm not sure this is a bug, but before it bites back too late, I'm
> reporting it now.
> 
> On pg_upgrading my catversion 201603111 9.6 cluster to 201603301, I
> got the following error:
> 
> -
>   pg_upgrade run on Fri Apr  1 22:50:07 2016
> -
> 
> Performing Consistency Checks
> -
> Checking cluster versions   ok
> Checking database user is the install user  ok
> Checking database connection settings   ok
> Checking for prepared transactions  ok
> Checking for reg* system OID user data typesok
> Checking for contrib/isn with bigint-passing mismatch   ok
> Creating dump of global objects ok
> Creating dump of database schemas
> 
> *failure*
> Consult the last few lines of "pg_upgrade_dump_13007.log" for
> the probable cause of the failure.
> 
> 
> command: "/usr/lib/postgresql/9.6/bin/pg_dump" --host 
> "/var/log/postgresql/pg_upgradecluster-9.6-9.6-main.lLkW" --port 5433 
> --username "postgres" --schema-only --quote-all-identifiers --binary-upgrade 
> --format=custom  --file="pg_upgrade_dump_13007.custom" "postgres" >> 
> "pg_upgrade_dump_13007.log" 2>&1
> pg_dump: [Archivierer (DB)] Anfrage fehlgeschlagen: FEHLER:  Spalte „amtype“ 
> existiert nicht
> ZEILE 1: SELECT tableoid, oid, amname, amtype, amhandler::pg_catalog
>^
> TIP:  Perhaps you meant to reference the column "pg_am.amname".
> pg_dump: [Archivierer (DB)] Anfrage war: SELECT tableoid, oid, amname, 
> amtype, amhandler::pg_catalog.regproc AS amhandler FROM pg_am

The reason for the failure is that pg_dump knows that 9.6 installations
have the amtype column -- but on your older devel 9.6 install, it
doesn't exist.  To fix it we would have to compare catalog versions in
pg_dump rather than major version, but we don't that anywhere.  Anyway
many upgrades could fail depending on other patches because of the same
reason.  I don't think we need to fix this.

-- 
Á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] pg_upgrade 9.6->9.6: column "amtype" does not exist

[Peter Geoghegan]

On Fri, Apr 1, 2016 at 2:31 PM, Christoph Berg  wrote:
> I'm not sure this is a bug, but before it bites back too late, I'm
> reporting it now.

This must be a regression from the changes made to the pg_am interface
by commit 65c5fcd353a859da9e61bfb2b92a99f12937de3b.

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

[HACKERS] pg_upgrade 9.6->9.6: column "amtype" does not exist

[Christoph Berg]

Hi,

I'm not sure this is a bug, but before it bites back too late, I'm
reporting it now.

On pg_upgrading my catversion 201603111 9.6 cluster to 201603301, I
got the following error:

-
  pg_upgrade run on Fri Apr  1 22:50:07 2016
-

Performing Consistency Checks
-
Checking cluster versions   ok
Checking database user is the install user  ok
Checking database connection settings   ok
Checking for prepared transactions  ok
Checking for reg* system OID user data typesok
Checking for contrib/isn with bigint-passing mismatch   ok
Creating dump of global objects ok
Creating dump of database schemas

*failure*
Consult the last few lines of "pg_upgrade_dump_13007.log" for
the probable cause of the failure.


command: "/usr/lib/postgresql/9.6/bin/pg_dump" --host 
"/var/log/postgresql/pg_upgradecluster-9.6-9.6-main.lLkW" --port 5433 
--username "postgres" --schema-only --quote-all-identifiers --binary-upgrade 
--format=custom  --file="pg_upgrade_dump_13007.custom" "postgres" >> 
"pg_upgrade_dump_13007.log" 2>&1
pg_dump: [Archivierer (DB)] Anfrage fehlgeschlagen: FEHLER:  Spalte „amtype“ 
existiert nicht
ZEILE 1: SELECT tableoid, oid, amname, amtype, amhandler::pg_catalog
   ^
TIP:  Perhaps you meant to reference the column "pg_am.amname".
pg_dump: [Archivierer (DB)] Anfrage war: SELECT tableoid, oid, amname, amtype, 
amhandler::pg_catalog.regproc AS amhandler FROM pg_am


'FEHLER...' translates to 'ERROR:  Column "amtype" does not exist'.


Christoph


-- 
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] More stable query plans via more predictable column statistics

[Tom Lane]

"Shulgin, Oleksandr"  writes:
> Alright.  I'm attaching the latest version of this patch split in two
> parts: the first one is NULLs-related bugfix and the second is the
> "improvement" part, which applies on top of the first one.

I've applied the first of these patches, broken into two parts first
because it seemed like there were two issues and second because Tomas
deserved primary credit for one part, ie realizing we were using the
Haas-Stokes formula wrong.

As for the other part, I committed it with one non-cosmetic change:
I do not think it is right to omit "too wide" values when considering
the threshold for MCVs.  As submitted, the patch was inconsistent on
that point anyway since it did it differently in compute_distinct_stats
and compute_scalar_stats.  But the larger picture here is that we define
the MCV population to exclude nulls, so it's reasonable to consider a
value as an MCV even if it's greatly outnumbered by nulls.  There is
no such exclusion for "too wide" values; those things are just an
implementation limitation in analyze.c, not something that is part of
the pg_statistic definition.  If there are a lot of "too wide" values
in the sample, we don't know whether any of them are duplicates, but
we do know that the frequencies of the normal-width values have to be
discounted appropriately.

Haven't looked at 0002 yet.

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

[Andres Freund]

On April 1, 2016 10:25:51 PM GMT+02:00, Jesper Pedersen 
 wrote:
>Hi,
>
>On 03/31/2016 06:21 PM, Andres Freund wrote:
>> On March 31, 2016 11:13:46 PM GMT+02:00, Jesper Pedersen
> wrote:
>>
>>> I can do a USE_CONTENT_LOCK run on 0003 if it is something for 9.6.
>>
>> Yes please. I think the lock variant is realistic, the lockless did
>isn't.
>>
>
>I have done a run with -M prepared on unlogged running 10min per data 
>point, up to 300 connections. Using data + wal on HDD.
>
>I'm not seeing a difference between with and without USE_CONTENT_LOCK
>-- 
>all points are within +/- 0.5%.
>
>Let me know if there are other tests I can perform

How do either compare to just 0002 applied?

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

[Jesper Pedersen]

Hi,

On 03/31/2016 06:21 PM, Andres Freund wrote:

On March 31, 2016 11:13:46 PM GMT+02:00, Jesper Pedersen 
 wrote:


I can do a USE_CONTENT_LOCK run on 0003 if it is something for 9.6.


Yes please. I think the lock variant is realistic, the lockless did isn't.



I have done a run with -M prepared on unlogged running 10min per data 
point, up to 300 connections. Using data + wal on HDD.


I'm not seeing a difference between with and without USE_CONTENT_LOCK -- 
all points are within +/- 0.5%.


Let me know if there are other tests I can perform.

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] Fix handling of invalid sockets returned by PQsocket()

[Alvaro Herrera]

Michael Paquier wrote:

> Here is v3 then, switching to "invalid socket" for those error
> messages. There are two extra messages in fe-misc.c and
> libpqwalreceiver.c that need a rewording that I have detected as well.

Peter Eisentraut pushed this as a40814d7a.

-- 
Á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] pgbench - remove unused clientDone parameter

[Alvaro Herrera]

Fabien wrote:
> 
> Remove pgbench clientDone unused "ok" parameter.

Seems useless, yeah, removed.

> I cannot get the point of keeping a useless parameter, which is probably
> there because at some point in the past it was used. If it is needed some
> day it can always be reinserted.

Actually it was introduced as an unused argument in 3da0dfb4b146.  I
don't quite see the reason, I guess they thought it'd become used at
some point.  Since it hasn't in the 7 intervening years, it seems safe
to remove 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] Re: [COMMITTERS] pgsql: Enable logical slots to follow timeline switches

[Alvaro Herrera]

Craig Ringer wrote:

> Note that I can't use PG_GETARG_TRANSACTIONID directly since it's a macro
> defined only in xid.c . It didn't seem worth extracting it and moving it to
> postgres.h (where the other non-ADT-specific PG_GETARG_ macros are) or its
> own new header just for this, so I've spelled it out each time.

Hm, we should probably fix this sometime.  We used to think of Xids are
purely internal objects, but they are becoming more and more visible to
userland.

> I now remember that that's part of why I used bigint as an argument type.
> The other part is that txid_current() returns bigint and there's no cast
> from bigint to xid. So the tests would have to CREATE CAST or cast via
> 'text'. They now do the latter.

I was rather surprised at this -- I thought that it'd break when the Xid
epoch got further than 0 (because casting from a number larger than 2^32
would throw an overflow error, I thought), but as it turns out the cast
from text to XID automatically discards the higher-order bits when a
higher epoch is used.  I imagine this is intentional, but it's not
actually documented in xidin() at all.  Also, I'm not sure what happens
if strtoul() refuses to work on > INT_MAX values ... it's documented to
set errno to ERANGE, so I assume the whole chain simply fails.  Maybe
there's just no platform on which that happens anymore.


With regards to the rest of the patch, I decided not to use your
approach: it seemed a bit odd to accept NULL values there.  I changed
the query to use COALESCE in the value that returned null instead.  With
your change, the function takes a null and interprets it as InvalidXid
anyway, so it seems to work fine.  (I also tested it manually.)

Let's see what hamster has to say about this.

It would be really good to have other buildfarm members run this code.
Currently only Hamster does, and only because Michaël patched the
buildfarm script ...

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

[Robbie Harwood]

Michael Paquier  writes:

> On Fri, Apr 1, 2016 at 12:31 PM, Robbie Harwood  wrote:
>
>> - Fixed buffering of large replies on the serverside.  This should fix
>>   the traceback that was being seen.  The issue had to do with the
>>   difference between the server and client calling conventions for the
>>   _read and _write functions.
>
> This does not fix the issue. I am still able to crash the server with
> the same trace.

Interesting.  I guess I'll keep trying to reproduce this.

>> - Move gss_encrypt out of the GUCs and into connection-specific logic.
>>   Thanks to Tom Lane for pointing me in the right direction here.
>
> +   /* delay processing until after AUTH_REQ_OK has been sent */
> +   if (port->gss->gss_encrypt != NULL)
> +   port->gss->encrypt = !strcmp(port->gss->gss_encrypt, "on");
>
> You should use parse_bool here, because contrary to libpq, clients
> should be able to use other values like "1", "0", "off", 'N', 'Y',
> etc.

Missed that function somehow.  Will fix.

>> - Replace writev() with two calls to _raw_write().  I'm not attached to
>>   this design; if someone has a preference for allocating a buffer and
>>   making a single write from that, I could be persuaded.  I don't know
>>   what the performance tradeoffs are.
>
> Relying on pqsecure_raw_write and pqsecure_raw_read is a better bet
> IMO, there is already some low-level error handling.

Indeed, it looks like it should especially lead to better behavior on
Windows.

>  static ConfigVariable *ProcessConfigFileInternal(GucContext context,
>   bool applySettings, int elevel);
>
> -
>  /*
>
> Spurious noise.

Indeed, will fix.


signature.asc
Description: PGP signature

Re: [HACKERS] syntax sugar for conditional check

[Tom Lane]

Jim Nasby  writes:
> Rather than this, I think an exclusive-or operator would be a lot more 
> useful. The only difficulty I run into with CHECK constaints is when I 
> want to ensure that only ONE condition is true.

"bool != bool" works as XOR.  If you need "exactly one of N" you could
do something like "(cond1::int + cond2::int + ...) = 1".  We could
wrap some syntactic sugar around either of these, but it's not clear
to me that it'd be any more useful than a custom SQL function.

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] syntax sugar for conditional check

[Pavel Stehule]

2016-04-01 18:57 GMT+02:00 David G. Johnston :

> On Thu, Mar 31, 2016 at 10:19 AM, Alexander Ostrow  wrote:
>
>> Hello,
>>
>> I thought it would be cool to have conditional check syntax, which gets
>> converted to simple check constraint syntax.
>>
>> Here’s a gist:
>>
>> https://gist.github.com/aj0strow/5a07f2ddcad324c4dac2c4095c821999
>>
>> It’s just sugar, but i think it would make check constraints easier to
>> read, and easier to write without backwards boolean logic.
>>
>
> For future reference please make every effort to make emails to the list
> self-contained - which has the added benefit of avoid link expiration in
> the future.
>
> As to the recommendation at hand - I don't see significant value in
> implementing non-SQL Standard syntax in this area.
>

+1

This formula should be known to all developers

Regards

Pavel


> David J.
> ​​
>
>

Re: [HACKERS] PQsendQuery+PQgetResult+PQsetSingleRowMode limitations and support

[Karl O. Pinc]

On Fri, 1 Apr 2016 05:57:33 +0200
"Shulgin, Oleksandr"  wrote:

> On Apr 1, 2016 02:57, "Karl O. Pinc"  wrote:
> >
> > I assume there are no questions about supporting a
> > similar functionality only without PQsetSingleRowMode,
> > as follows:  
> 
> Sorry, but I don't see what is your actual question here?

The question is whether or not the functionality of the first
script is supported.  I ask since Bruce was surprised to see
this working and questioned whether PG was intended to behave
this way.

> Both code examples are going to compile and work, AFAICS. The
> difference is that the latter will try to fetch the whole result set
> into client's memory before returning you a PGresult.

Thanks for the clarification.  For some reason I recently
got it into my head that the libpq buffering was on the server side,
which is really strange since I long ago determined it was
client side.




Karl 
Free Software:  "You don't pay back, you pay forward."
 -- Robert A. Heinlein


-- 
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] syntax sugar for conditional check

[Jim Nasby]

On 3/31/16 12:19 PM, Alexander Ostrow wrote:

Hello,

I thought it would be cool to have conditional check syntax, which gets
converted to simple check constraint syntax.

Here’s a gist:

https://gist.github.com/aj0strow/5a07f2ddcad324c4dac2c4095c821999

It’s just sugar, but i think it would make check constraints easier to
read, and easier to write without backwards boolean logic.


Rather than this, I think an exclusive-or operator would be a lot more 
useful. The only difficulty I run into with CHECK constaints is when I 
want to ensure that only ONE condition is true.

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


--
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] Phrase search ported to 9.6

[Oleg Bartunov]

On Fri, Apr 1, 2016 at 5:24 PM, Tom Lane  wrote:

> Alvaro Herrera  writes:
> > Teodor Sigaev wrote:
> >> may be <=>? it isn't used anywhere yet.
> >>
> >> select 'fat'::tsquery <=> 'cat';
> >> select 'fat <=> cat'::tsquery;
> >> select 'fat <3> cat'::tsqyery; -- for non-default distance.
>
> > Dunno.  That looks pretty "relationalish".
>
> The existing precedent in the geometric types is to use <->
> as an operator denoting distance.  Perhaps <-> amd <3>
> would work here.
>
>
looks like we get consensus about arrows.


> regards, tom lane
>

[HACKERS] Re: [COMMITTERS] pgsql: Improve internationalization of messages involving type names

[Alvaro Herrera]

Moving to -hackers.

Tom Lane wrote:
> Alvaro Herrera  writes:
> > Alvaro Herrera wrote:
> >> Tom Lane wrote:
> >>> Actually, I think the general convention is to NOT quote type names
> >>> in error messages.
> 
> >> Ok, I'll change it.
> 
> > Done.
> 
> Thanks.  Should we make a push to get rid of hard-wired type names in
> other messages?  There's still quite a few "invalid input syntax for ..."
> instances that could be whacked until they all share the same translatable
> message.

Ah, I hadn't noticed those precisely because they lack the quotes ... I
grepped for 'type \"%' and found a few matches but didn't seem
interesting enough since there's only one of each, such as
errmsg("constant of the type \"regrole\" cannot be used here")

The 'invalid input syntax' are much more numerous.  Also the "value XYZ
is out of range" messages.  Both of those seem definitely worth fixing.
I also saw "value too long" for char/varchar but they're only two
distinct messages and fixing those would require printing the maxlen in
place before raising the message -- probably not worth the trouble.

I also found these while paging through grep results,

src/backend/utils/adt/json.c:errmsg("could not determine 
data type for argument 1")));
src/backend/utils/adt/json.c:errmsg("could not determine 
data type for argument 2")));
src/backend/utils/adt/json.c:errmsg("could not determine 
data type for argument %d",

which seem worth merging too.

If nobody opposes, I will fix those after feature freeze.

-- 
Á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] syntax sugar for conditional check

[David G. Johnston]

On Thu, Mar 31, 2016 at 10:19 AM, Alexander Ostrow  wrote:

> Hello,
>
> I thought it would be cool to have conditional check syntax, which gets
> converted to simple check constraint syntax.
>
> Here’s a gist:
>
> https://gist.github.com/aj0strow/5a07f2ddcad324c4dac2c4095c821999
>
> It’s just sugar, but i think it would make check constraints easier to
> read, and easier to write without backwards boolean logic.
>

For future reference please make every effort to make emails to the list
self-contained - which has the added benefit of avoid link expiration in
the future.

As to the recommendation at hand - I don't see significant value in
implementing non-SQL Standard syntax in this area.

David J.
​​

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

[Tom Lane]

David Rowley  writes:
> On 12 March 2016 at 11:43, Tom Lane  wrote:
>> It seems like the major intellectual complexity here is to figure out
>> how to detect inner-side-unique at reasonable cost.  I see that for
>> LEFT joins you're caching that in the SpecialJoinInfos, which is probably
>> fine.  But for INNER joins it looks like you're just doing it over again
>> for every candidate join, and that seems mighty expensive.

> I have a rough idea for this, but I need to think of it a bit more to
> make sure it's bulletproof.

Where are we on this?  I had left it alone for awhile because I supposed
that you were going to post a new patch, but it's been a couple weeks...

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] So, can we stop supporting Windows native now?

[Michael Banck]

On Thu, Mar 31, 2016 at 09:04:35AM +0800, Craig Ringer wrote:
> If we eventually get a CMake build system conversion that'll mostly go
> away too.

Well, maybe the good message about this new development is that
autotools will start working much better on Windows and could be
eventually used for the Windows port as well, making a CMake conversion
moot.


Michael


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

[HACKERS] syntax sugar for conditional check

[Alexander Ostrow]

Hello,

I thought it would be cool to have conditional check syntax, which gets 
converted to simple check constraint syntax.

Here’s a gist:

https://gist.github.com/aj0strow/5a07f2ddcad324c4dac2c4095c821999

It’s just sugar, but i think it would make check constraints easier to read, 
and easier to write without backwards boolean logic.

Thank you,

AJ

Re: [HACKERS] raw output from copy

[Daniel Verite]

Andrew Dunstan wrote:

> If someone can make a good case that this is going to be of
> general use I'll happily go along, but I haven't seen one so far.

About COPY FROM with a raw format, for instance just yesterday
there was this user question on stackoverflow:
http://stackoverflow.com/questions/36317237

which essentially is: how to import contents from a file without any
particular interpretation of any character?

With the patch discussed in this thread, a user can do
\copy table(textcol) from /path/to/file (format raw)
or the equivalent COPY.
If it's a binary column, that works just the same.

Without this, it's not obvious at all how this result can be
achieved without resorting to external preprocessing,
and assuming the availability of such preprocessing tools
in the environment. Notwithstanding the fact that the
solution proposed on SO (doubling backslashes with sed)
doesn't even work if the file contains tabs, as they would be
interpreted as field separators, even if the copy target has only
one column. You can change the delimiter with COPY but AFAIK
you can't tell that there is none.

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


-- 
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] OOM in libpq and infinite loop with getCopyStart()

[Tom Lane]

I wrote:
> So the core of my complaint is that we need to fix things so that, whether
> or not we are able to create the PGRES_FATAL_ERROR PGresult (and we'd
> better consider the behavior when we cannot), ...

BTW, the real Achilles' heel of any attempt to ensure sane behavior at
the OOM limit is this possibility of being unable to create a PGresult
with which to inform the client that we failed.

I wonder if we could make things better by keeping around an emergency
backup PGresult struct.  Something along these lines:

1. Add a field "PGresult *emergency_result" to PGconn.

2. At the very beginning of any PGresult-returning libpq function, check
to see if we have an emergency_result, and if not make one, ensuring
there's room in it for a reasonable-size error message; or maybe even
preload it with "out of memory" if we assume that's the only condition
it'll ever be used for.  If malloc fails at this point, just return NULL
without doing anything or changing any libpq state.  (Since a NULL result
is documented as possibly caused by OOM, this isn't violating any API.)

3. Subsequent operations never touch the emergency_result unless we're
up against an OOM, but it can be used to return a failure indication
to the client so long as we leave libpq in a state where additional
calls to PQgetResult would return NULL.

Basically this shifts the point where an unreportable OOM could happen
from somewhere in the depths of libpq to the very start of an operation,
where we're presumably in a clean state and OOM failure doesn't leave
us with a mess we can't clean up.

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] OOM in libpq and infinite loop with getCopyStart()

[Tom Lane]

Amit Kapila  writes:
> Very valid point.  So, if we see with patch, I think libpq will be
> in PGASYNC_COPY_XXX state after such a failure and next time when it tries
> to again execute statement, it will end copy mode and then allow to proceed
> from there.   This design is required for other purposes like silently
> discarding left over result.  Now, I think the other option(if possible)
> seems to be that we can put libpq in PGASYNC_IDLE, if we get such an error
> as we do at some other places in case of error as below and return OOM
> failure as we do in patch.

If we transition to PGASYNC_IDLE then the next PQexecStart will not
realize that it needs to do something to get out of the COPY state;
but it does, since the backend thinks that we're doing COPY.  That was
the reasoning behind my proposal to invent new PGASYNC states.
Obviously there's more than one way to represent this new state, eg
you could have a separate error flag instead --- but it is a new state.

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] Logical Decoding - Execute join query

[Craig Ringer]

On 1 April 2016 at 17:45, Andres Freund  wrote:

> Hi,
>
> On 2016-04-01 15:09:59 +0530, hari.prasath wrote:
> >   I tried to execute a join query using SPI_execute() in logical
> >   decoding part and got inconsistent values (i am referring it as
> >   inconsistent since it is returning the old values which is
> >   present at the postgresql server start).
>
> You are not allowed to access non catalog tables in an output plugin. To
> quote the manual:
> > Read only access to relations is permitted as long as only relations are
> > accessed that either have been created by initdb in
> > the pg_catalog schema, or have been marked as user
> > provided catalog tables using


> The reason for that is that we'd have to keep all rows in the tables, if
> you wanted to be look at the state "in the past".
>

I suspect this is going to come up more and more as people start using
logical decoding.

A while back I had a quick look at ways to ensure we actually die with an
assertion failure when this happens. I didn't have much luck. The places I
could find where something definitely unsafe would be happening were too
far from anywhere that had knowledge of the relation's catalog entry to
check whether it was a user catalog. Not without doing relcache lookups
just to check an assertion, anyway. Or to necessarily even know it was
running under a historical snapshot without poking through layers messily.
OTOH, I don't know the area well and didn't dig too deeply.

Then again, IIRC the SPI still lets you proceed in read-only mode without
having a snapshot set up or an open xact... and it might work. For a while.
Sometimes. Possibly even with correct results. Depending on what exactly
you do. The world doesn't seem to have ended as a result of not immediately
dying with an assertion failure in that situation.

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

Re: [HACKERS] OOM in libpq and infinite loop with getCopyStart()

[Tom Lane]

Michael Paquier  writes:
> On Fri, Apr 1, 2016 at 12:48 PM, Tom Lane  wrote:
>> Anyway, the short of my review is that we need more clarity of thought
>> about what state libpq is in after a failure like this, and what that
>> state looks like to the application, and how that should affect how
>> libpq reacts to application calls.

> Hm. I would have thought that returning to the caller a result
> generated by PQmakeEmptyPGresult(PGRES_FATAL_ERROR) with an error
> string marked "out of memory" would be the best thing to do instead of
> NULL...

Sure, we're trying to do that, but the question is what happens afterward.
An app that is following the documented usage pattern for PQgetResult
will call it again, expecting to get a NULL, but as the patch stands it
might get a PGRES_COPY_XXX instead.  What drove me to decide the patch
wasn't committable was realizing that Robert was right upthread: the
change you propose in libpqwalreceiver.c is not a bug fix.  That code
is doing exactly what it's supposed to do per the libpq.sgml docs, namely
iterate till it gets a NULL.[1]  If we have to change it, that means
(a) we've changed the API spec for PQgetResult and (b) every other
existing caller of PQgetResult would need to change similarly, or else
risk corner-case bugs about as bad as the one we're trying to fix.

So the core of my complaint is that we need to fix things so that, whether
or not we are able to create the PGRES_FATAL_ERROR PGresult (and we'd
better consider the behavior when we cannot), we need to leave libpq
in a state where subsequent calls to PQgetResult will return NULL.

> If getCopyStart() complains because of a lack of data, we loop
> back into pqParseInput3 to try again. If an OOM happens, we still loop
> into pqParseInput3 but all the pending data received from client needs
> to be discarded so as we don't rely on the next calls of PQexecStart
> to do the cleanup, once all the data is received we get out of the
> loop and generate the result with PGRES_FATAL_ERROR. Does that match
> what you have in mind?

If we could, that would be OK, but (a) you're only considering the
COPY OUT case not the COPY IN case, and (b) a persistent OOM condition
could very well prevent us from ever completing the copy.  For example,
some COPY DATA messages might be too big for our current buffer, but
we won't be able to enlarge the buffer.  I'm inclined to think that
reporting the OOM to the client is the highest-priority goal, and
that attempting to clean up during the next PQexec/PQsendQuery is a
reasonable design.  If we fail to do that, the client won't really
understand that it's a cleanup failure and not a failure to issue the
new query, but it's not clear why it needs to know the difference.

regards, tom lane

[1] Well, almost.  Really, after dealing with a PGRES_COPY_XXX result
it ought to go back to pumping PQgetResult.  It doesn't, but that's
an expected client behavior, and cleaning up after that is exactly what
PQexecStart is intended for.


-- 
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] dealing with extension dependencies that aren't quite 'e'

[Alvaro Herrera]

Abhijit Menon-Sen wrote:
> At 2016-03-29 10:15:51 -0400, da...@pgmasters.net wrote:
> >
> > Either way it looks like you need to post a patch with more
> > documentation - do you know when you'll have that ready?
> 
> Here it is.
> 
> (I was actually looking for other potential callers, but I couldn't find
> any. There are some places that take a RangeVar and make a list from it,
> but they are creating new nodes, and don't quite fit. So the only change
> in this patch is to add a comment above the get_object_address_rv
> function.)
> 
> Álvaro, do you like this one any better?

Well, yes and no.  It's certainly much cleaner than the other approach
IMO.  But this patch makes me consider the following question: could I
use this to implement ExecRenameStmt, instead of the current code?  It's
not a trivial question, because the current rename code goes through
RangeVarGetRelidExtended with custom callbacks, to ensure that they have
the correct object before locking.  get_object_address also has some
protections against this, but I'm not really clear on whether they offer
the same guarantees.  If they do, we could replace the rangevar lookup
with the new get_object_address_rv and the end result would probably
turn out simpler.

At this point I hate myself for introducing the SQL-accessible code for
get_object_address and friends; we could change the representation of
what comes out of the parser, but that functionality would be broken if
we do it now.  I think it's okay to change it at some point in the
future, given sufficient reason, but I'm not sure that this patch is
that.

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

[Alvaro Herrera]

Kevin Grittner wrote:

> Attached is what I think you're talking about for the first patch.
> AFAICS this should generate identical executable code to unpatched.
> Then the patch to actually implement the feature would, instead
> of adding 30-some lines with TestForOldSnapshot() would implement
> that as the behavior for the other enum value, and alter those
> 30-some BufferGetPage() calls.
> 
> Álvaro and Michael, is this what you were looking for?

Yes, this is what I was thinking, thanks.

-- 
Á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] Phrase search ported to 9.6

[Tom Lane]

Alvaro Herrera  writes:
> Teodor Sigaev wrote:
>> may be <=>? it isn't used anywhere yet.
>> 
>> select 'fat'::tsquery <=> 'cat';
>> select 'fat <=> cat'::tsquery;
>> select 'fat <3> cat'::tsqyery; -- for non-default distance.

> Dunno.  That looks pretty "relationalish".

The existing precedent in the geometric types is to use <->
as an operator denoting distance.  Perhaps <-> amd <3>
would work here.

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] SSL indicator in psql prompt

[Tom Lane]

Peter Eisentraut  writes:
> I like how browsers show a little lock in the address bar depending on
> whether SSL is in use.  This could be useful in psql as well.  Here is a
> prototype patch.
> Comments?

-1 on the hard-coded UTF8, even with the encoding check (which I don't
think is terribly trustworthy).  How about defining it in a way that
lets/makes the user provide the character(s) to print?

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: Access method extendability

[Aleksander Alekseev]

Hello

> Fixed.

Thanks. I don't have any other suggestions at the moment. Let see whats
committers opinion on this.

-- 
Best regards,
Aleksander Alekseev
http://eax.me/


-- 
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 Apr 1, 2016, at 10:04 AM, Michael Paquier  
> wrote:
> 
> I would suggest the following name modifications, node names have been
> introduced to help tracking of each node's log:
> - Candie => master
> - Django => slave or just standby
> There is no need for complication! And each node's log file is
> prefixed by the test number. Note that in other tests there are no
> promotions, or fallbacks done, but we stick with a node name that
> represents the initial state of the cluster.

Ok, let’s reflect initial state in node names. So master and standby then.

> 
>> +# Switch to synchronous replication
>> +$node_master->append_conf('postgresql.conf', qq(
>> +synchronous_standby_names = '*'
>> +));
>> +$node_master->restart;
>> Reloading would be fine.
>> 
>> Good catch, done.
> 
> +$node_master->psql('postgres', "select pg_reload_conf()");
> 
> It would be cleaner to introduce a new routine in PostgresNode that
> calls pg_ctl reload (mentioned in the N-sync patch as well, that would
> be useful for many purposes).

Okay.

> 
>> All accesses to 2pc dummies in ProcArray are covered with TwoPhaseStateLock,
>> so I thick that’s safe. Also I’ve deleted comment above that block, probably
>> it’s more confusing than descriptive.
> 
> OK, you removed the use to allProcs. Though by reading again the code
> just holding TwoPhaseStateLock that's actually fine.
> 
> The patch needs a small cleanup:
> $ git diff master --check
> src/test/recovery/t/006_twophase.pl:224: new blank line at EOF.
> 
> 006_twophase.pl should be renamed to 007. It keeps its license to
> kill, and gains in being famous.

Huh, eventually there will be Fleming reference, instead of Tarantino one in 
node names.

> 
> - * Scan the pg_twophase directory and setup all the required information to
> - * allow standby queries to treat prepared transactions as still active.
> - * This is never called at the end of recovery - we use
> - * RecoverPreparedTransactions() at that point.
> + * It's a caller responsibility to call MarkAsPrepared() on returned gxact.
>  *
> Wouldn't it be more simple to just call MarkAsPrepared at the end of
> RecoverPreparedFromBuffer?

I did that intentionally to allow modification of gxact before unlock.

RecoverPreparedFromXLOG:
gxact = RecoverPreparedFromBuffer((char *) XLogRecGetData(record), 
false);
gxact->prepare_start_lsn = record->ReadRecPtr;
gxact->prepare_end_lsn = record->EndRecPtr;
MarkAsPrepared(gxact);

RecoverPreparedFromFiles:
gxact = RecoverPreparedFromBuffer(buf, forceOverwriteOK);
gxact->ondisk = true;
MarkAsPrepared(gxact);

While both function are only called during recovery, I think that it is better 
to write that
in a way when it possible to use it in multiprocess environment.

> 
> While testing the patch, I found a bug in the recovery conflict code
> path. You can do the following to reproduce it:
> 1) Start a master with a standby
> 2) prepare a transaction on master
> 3) Stop immediate on standby to force replay
> 4) commit prepare transaction on master
> 5) When starting the standby, it remains stuck here:

Hm, I wasn’t able to reproduce that. Do you mean following scenario or am I 
missing something?

(async replication)

$node_master->psql('postgres', "
begin;
insert into t values (1);
prepare transaction 'x';
");
$node_slave->teardown_node;
$node_master->psql('postgres',"commit prepared 'x'");
$node_slave->start;
$node_slave->psql('postgres',"select count(*) from pg_prepared_xacts", stdout 
=> \$psql_out);
is($psql_out, '0', "Commit prepared on master while slave is down.");


> * thread #1: tid = 0x229b4, 0x7fff8e2d4f96
> libsystem_kernel.dylib`poll + 10, queue = 'com.apple.main-thread',
> stop reason = signal SIGSTOP
>  * frame #0: 0x7fff8e2d4f96 libsystem_kernel.dylib`poll + 10
>frame #1: 0x000107e5e043
> postgres`WaitEventSetWaitBlock(set=0x7f90c20596a8, cur_timeout=-1,
> occurred_events=0x7fff581efd28, nevents=1) + 51 at latch.c:1102
>frame #2: 0x000107e5da26
> postgres`WaitEventSetWait(set=0x7f90c20596a8, timeout=-1,
> occurred_events=0x7fff581efd28, nevents=1) + 390 at latch.c:935
>frame #3: 0x000107e5d4c7
> postgres`WaitLatchOrSocket(latch=0x000111432464, wakeEvents=1,
> sock=-1, timeout=-1) + 343 at latch.c:347
>frame #4: 0x000107e5d36a
> postgres`WaitLatch(latch=0x000111432464, wakeEvents=1, timeout=0)
> + 42 at latch.c:302
>frame #5: 0x000107e7b5a6 postgres`ProcWaitForSignal + 38 at proc.c:1731
>frame #6: 0x000107e6a4eb
> postgres`ResolveRecoveryConflictWithLock(locktag=LOCKTAG at
> 0x7fff581efde8) + 187 at standby.c:391
>frame #7: 0x000107e7a6a8
> postgres`ProcSleep(locallock=0x7f90c203dac8,
> lockMethodTable=0x0001082f6218) + 1128 at proc.c:1215
>frame #8: 0x000107e72886
> postgres`WaitOnLock(locallock=0x7f90c203dac8,
> owner=0x) + 358 at 

Re: [HACKERS] [PATCH] Phrase search ported to 9.6

[Andreas Joseph Krogh]

På fredag 01. april 2016 kl. 15:22:55, skrev Teodor Sigaev >:
> there was a character that was very similar to dots I would suggest
 > that.  The closest is * I think, so what do you think of "***"?

 And join opertator for tsqueries is the same :
 select 'fat'::tsquery *** 'cat'; ?

 Single '*' ?  That's close to regex, any number of tokens. And it saves rules
 about duplicating character.

 select 'fat'::tsquery ** 'cat';
 select 'fat * cat'::tsquery;
 select 'fat * [3] cat'::tsqyery; -- for non-default distance.
 
What about ~> ?
 
-- Andreas Joseph Krogh

Re: [HACKERS] SSL indicator in psql prompt

[Shulgin, Oleksandr]

On Fri, Apr 1, 2016 at 2:52 PM, Peter Eisentraut  wrote:

> I like how browsers show a little lock in the address bar depending on
> whether SSL is in use.  This could be useful in psql as well.  Here is a
> prototype patch.
>
> Example:
>
> Put this in .psqlrc:
>
> \set PROMPT1 '%s%/%R%# '
>
> $ psql test
> psql (9.6devel)
> Type "help" for help.
>
> test=#
>
> Without SSL:
>
> test=#
>
> Comments?
>

Sounds reasonable.  What is the meaning of the latter symbol?

--
Alex

Re: [HACKERS] pg_xlogdump fails to handle WAL file with multi-page XLP_FIRST_IS_CONTRECORD data

[Craig Ringer]

On 31 March 2016 at 16:13, Andres Freund  wrote:


> It's probably easier to just generate a humongous commit record. You can
> do so by having a *lot* of subtransactions. Relatively easy to do with
> plpgsql by creating them in a loop (SELECT txid_current() in EXCEPTION
> bearing block ought to suffice).
>
>
This does the trick and does it quickly on 9.4:

CREATE TABLE trivial(padding text);
ALTER TABLE trivial ALTER COLUMN padding SET STORAGE plain;

DO
LANGUAGE plpgsql
$$
DECLARE
  wal_seg_size integer;
  remaining_size integer;
BEGIN
  wal_seg_size := (select setting from pg_settings where name =
'wal_segment_size')::integer
* (select setting from pg_settings where name =
'wal_block_size')::integer;
  LOOP
SELECT wal_seg_size - file_offset FROM
pg_xlogfile_name_offset(pg_current_xlog_insert_location()) INTO
remaining_size;
IF remaining_size < 8192
THEN
-- Make a nice big commit record right at the end of the segment
EXIT;
ELSE
BEGIN
  -- About 4k
  INSERT INTO trivial(padding) VALUES (repeat('0123456789abcdef',
256));
EXCEPTION
  WHEN division_by_zero THEN
CONTINUE;
END;
END IF;
  END LOOP;
END;
$$;


I had no issue reproducing the bug on 9.4, but I don't see it in 9.6. At
least, not this one, not yet.

Still, we might want to backpatch the fix; it seems clear enough even if I
can't reproduce the issue yet. It doesn't look like it'll affect logical
decoding since the snapshot builder has its own unrelated logic for finding
the startpoint for decoding, and it certainly doesn't affect WAL replay
otherwise we would've seen the fireworks much earlier. The upside is that
also makes the backpatch much safer.

I was surprised to see that there are no tests for pg_xlogdump to add this
on to, so I've added a trivial test. I've got some more complete
xlogdump-helper code in the failover slots tests that I should extract and
add to PostgresNode.pm but this'll do in the mean time and is much simpler.

In the process I noticed that pg_xlogdump doesn't follow the rules of the
rest of the codebase when it comes to command line handling with --version,
returning nonzero on bad options, etc. It might be good to fix that;
there's a small BC impact, but I doubt anyone cares when it comes to
pg_xlogdump.

I'll attach the new testcase once I either get it to reproduce this bug or
give up and leave the basic xlogdump testcase alone.


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

Re: [HACKERS] [PATCH] Phrase search ported to 9.6

[Teodor Sigaev]

there was a character that was very similar to dots I would suggest
that.  The closest is * I think, so what do you think of "***"?


And join opertator for tsqueries is the same :
select 'fat'::tsquery *** 'cat'; ?

Single '*' ?  That's close to regex, any number of tokens. And it saves rules 
about duplicating character.


select 'fat'::tsquery ** 'cat';
select 'fat * cat'::tsquery;
select 'fat * [3] cat'::tsqyery; -- for non-default distance.


--
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] WIP: Access method extendability

[Alexander Korotkov]

Hi!

On Fri, Apr 1, 2016 at 12:45 PM, Aleksander Alekseev <
a.aleks...@postgrespro.ru> wrote:

> Code looks much better now, thanks. Still I believe it could be improved.
>
> I don't think that using srand() / rand() in signValue procedure the
> way you did is such a good idea. You create a side affect (changing
> current randseed) which could cause problems in some cases. And there
> is no real need for that. For instance you could use following formula
> instead:
>
> hash(attno || hashVal || j)
>

I've discussed this with Teodor privately.  Extra hash calculation could
cause performance regression.  He proposed to use own random generator
instead.  Implemented in attached version of patch.


> And a few more things.
>
> > + memset(opaque, 0, sizeof(BloomPageOpaqueData));
> > + opaque->maxoff = 0;
>


> This looks a bit redundant.
>

Fixed.


> > + for (my $i = 1; $i <= 10; $i++)
>
> More idiomatic Perl would be `for my $i (1..10)`.
>

Fixed.


> > + UnlockReleaseBuffer(buffer);
> > + ReleaseBuffer(metaBuffer);
> > + goto away;
>
> In general I don't have anything against goto. But are you sure that
> using it here is really justified?


Fixed with small code duplication which seems to be better than goto.

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


0003-bloom-contrib.16.patch
Description: Binary data

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

Re: [HACKERS] [PATCH] Phrase search ported to 9.6

[Alvaro Herrera]

Teodor Sigaev wrote:
> >Well, I noticed that the docs talk about an operator that can be used in
> >SQL (outside the tsquery parser), as well as an operator that can be
> Just to join 2 tsquery with operator FOLLOWED BY

Ok.

> >used inside tsquery.  Inside tsquery anything would be usable, but
> >outside that it would be good to keep in mind the rest of SQL operators;
> >and <> means "different from", so using it for FOLLOWED BY seems odd to
> >me.
> previous rule was: duplicate operation character to join tsqueries.
> Ellipsis is forbidden to use as operator name:
> #  create operator ... (procedure = 'int2and');
> ERROR:  syntax error at or near ".."
> LINE 1: create operator ... (procedure = 'int2and');
> What is your suggestion for joining operator name?

Silly me ...  The dot is not allowed in operator names :-(  Sorry.  If
there was a character that was very similar to dots I would suggest
that.  The closest is * I think, so what do you think of "***"?

> >My suggestion of ... (ellipsis) is because that's already known as
> >related to text, used for omissions of words, where the surrounding
> >words form a phrase.  It seems much more natural to me.
> Yes, agree for omission. But for reason above its'n a good name although I
> don't have a strong objections.
> 
> may be <=>? it isn't used anywhere yet.
> 
> select 'fat'::tsquery <=> 'cat';
> select 'fat <=> cat'::tsquery;
> select 'fat <3> cat'::tsqyery; -- for non-default distance.

Dunno.  That looks pretty "relationalish".

-- 
Á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] Phrase search ported to 9.6

[Teodor Sigaev]

Well, I noticed that the docs talk about an operator that can be used in
SQL (outside the tsquery parser), as well as an operator that can be

Just to join 2 tsquery with operator FOLLOWED BY


used inside tsquery.  Inside tsquery anything would be usable, but
outside that it would be good to keep in mind the rest of SQL operators;
and <> means "different from", so using it for FOLLOWED BY seems odd to
me.

previous rule was: duplicate operation character to join tsqueries.
Ellipsis is forbidden to use as operator name:
#  create operator ... (procedure = 'int2and');
ERROR:  syntax error at or near ".."
LINE 1: create operator ... (procedure = 'int2and');
What is your suggestion for joining operator name?


My suggestion of ... (ellipsis) is because that's already known as
related to text, used for omissions of words, where the surrounding
words form a phrase.  It seems much more natural to me.
Yes, agree for omission. But for reason above its'n a good name although I don't 
have a strong objections.


may be <=>? it isn't used anywhere yet.

select 'fat'::tsquery <=> 'cat';
select 'fat <=> cat'::tsquery;
select 'fat <3> cat'::tsqyery; -- for non-default distance.


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

[HACKERS] SSL indicator in psql prompt

[Peter Eisentraut]

I like how browsers show a little lock in the address bar depending on
whether SSL is in use.  This could be useful in psql as well.  Here is a
prototype patch.

Example:

Put this in .psqlrc:

\set PROMPT1 '%s%/%R%# '

$ psql test
psql (9.6devel)
Type "help" for help.

test=#

Without SSL:

test=#

Comments?
From b3a79d56507c61b6fa6efba6f97a37acf822d39f Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Fri, 1 Apr 2016 00:00:00 +
Subject: [PATCH] psql: Add %s prompt placeholder for SSL status

---
 doc/src/sgml/ref/psql-ref.sgml |  9 +
 src/bin/psql/prompt.c  | 20 
 2 files changed, 29 insertions(+)

diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 385cb59..b4044e2 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -3512,6 +3512,15 @@ Prompting
   
 
   
+%s
+
+ 
+  A character indicating whether SSL is in use.
+ 
+
+  
+
+  
 %digits
 
 
diff --git a/src/bin/psql/prompt.c b/src/bin/psql/prompt.c
index 647e871..3c25a2f 100644
--- a/src/bin/psql/prompt.c
+++ b/src/bin/psql/prompt.c
@@ -21,6 +21,7 @@
 #include "input.h"
 #include "prompt.h"
 #include "settings.h"
+#include "mb/pg_wchar.h"
 
 
 /*--
@@ -44,6 +45,7 @@
  *			or a ! if session is not connected to a database;
  *		in prompt2 -, *, ', or ";
  *		in prompt3 nothing
+ * %s - SSL mode
  * %x - transaction status: empty, *, !, ? (unknown or no connection)
  * %l - The line number inside the current statement, starting from 1.
  * %? - the error code of the last query (not yet implemented)
@@ -218,6 +220,24 @@ get_prompt(promptStatus_t status)
 	}
 	break;
 
+case 's':
+	if (pset.db && PQsslInUse(pset.db))
+	{
+		if (pset.encoding == PG_UTF8)
+			strlcpy(buf, "\xF0\x9F\x94\x92 ", sizeof(buf));  /* U+1F512 */
+		else
+			strlcpy(buf, "#", sizeof(buf));
+	}
+	else
+	{
+		if (pset.encoding == PG_UTF8)
+			strlcpy(buf, "\xF0\x9F\x83\x8F", sizeof(buf));  /* U+1F0CF */
+		else
+			strlcpy(buf, "_", sizeof(buf));
+
+	}
+	break;
+
 case 'x':
 	if (!pset.db)
 		buf[0] = '?';
-- 
2.8.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] dealing with extension dependencies that aren't quite 'e'

[Abhijit Menon-Sen]

At 2016-03-29 10:15:51 -0400, da...@pgmasters.net wrote:
>
> Either way it looks like you need to post a patch with more
> documentation - do you know when you'll have that ready?

Here it is.

(I was actually looking for other potential callers, but I couldn't find
any. There are some places that take a RangeVar and make a list from it,
but they are creating new nodes, and don't quite fit. So the only change
in this patch is to add a comment above the get_object_address_rv
function.)

Álvaro, do you like this one any better?

-- Abhijit
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 951f59b..189b771 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -2864,6 +2864,19 @@
   
  
 
+
+
+ DEPENDENCY_AUTO_EXTENSION (x)
+ 
+  
+   The dependent object is not a member of the extension that is the
+   referenced object (and so should not be ignored by pg_dump), but
+   cannot function without it and should be dropped when the
+   extension itself is. The dependent object may be dropped on its
+   own as well.
+  
+ 
+

 
Other dependency flavors might be needed in future.
diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c
index c48e37b..a284bed 100644
--- a/src/backend/catalog/dependency.c
+++ b/src/backend/catalog/dependency.c
@@ -587,6 +587,7 @@ findDependentObjects(const ObjectAddress *object,
 		{
 			case DEPENDENCY_NORMAL:
 			case DEPENDENCY_AUTO:
+			case DEPENDENCY_AUTO_EXTENSION:
 /* no problem */
 break;
 			case DEPENDENCY_INTERNAL:
@@ -786,6 +787,7 @@ findDependentObjects(const ObjectAddress *object,
 subflags = DEPFLAG_NORMAL;
 break;
 			case DEPENDENCY_AUTO:
+			case DEPENDENCY_AUTO_EXTENSION:
 subflags = DEPFLAG_AUTO;
 break;
 			case DEPENDENCY_INTERNAL:
diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c
index d2aaa6d..39128c1 100644
--- a/src/backend/catalog/objectaddress.c
+++ b/src/backend/catalog/objectaddress.c
@@ -999,6 +999,32 @@ get_object_address(ObjectType objtype, List *objname, List *objargs,
 }
 
 /*
+ * Return an ObjectAddress based on a RangeVar and an obect name. The
+ * name of the relation identified by the RangeVar is prepended to the
+ * list passed in as objname. This is useful to find the ObjectAddress
+ * of objects that depend on a relation. All other considerations are
+ * exactly as for get_object_address above.
+ */
+
+ObjectAddress
+get_object_address_rv(ObjectType objtype, RangeVar *rel, List *objname,
+	  List *objargs, Relation *relp, LOCKMODE lockmode,
+	  bool missing_ok)
+{
+	if (rel)
+	{
+		objname = lcons(makeString(rel->relname), objname);
+		if (rel->schemaname)
+			objname = lcons(makeString(rel->schemaname), objname);
+		if (rel->catalogname)
+			objname = lcons(makeString(rel->catalogname), objname);
+	}
+
+	return get_object_address(objtype, objname, objargs,
+			  relp, lockmode, missing_ok);
+}
+
+/*
  * Find an ObjectAddress for a type of object that is identified by an
  * unqualified name.
  */
diff --git a/src/backend/commands/alter.c b/src/backend/commands/alter.c
index 5af0f2f..dd0b4c9f 100644
--- a/src/backend/commands/alter.c
+++ b/src/backend/commands/alter.c
@@ -23,6 +23,7 @@
 #include "catalog/pg_collation.h"
 #include "catalog/pg_conversion.h"
 #include "catalog/pg_event_trigger.h"
+#include "catalog/pg_extension.h"
 #include "catalog/pg_foreign_data_wrapper.h"
 #include "catalog/pg_foreign_server.h"
 #include "catalog/pg_language.h"
@@ -32,6 +33,7 @@
 #include "catalog/pg_opclass.h"
 #include "catalog/pg_opfamily.h"
 #include "catalog/pg_proc.h"
+#include "catalog/pg_trigger.h"
 #include "catalog/pg_ts_config.h"
 #include "catalog/pg_ts_dict.h"
 #include "catalog/pg_ts_parser.h"
@@ -391,6 +393,32 @@ ExecRenameStmt(RenameStmt *stmt)
 }
 
 /*
+ * Executes an ALTER OBJECT / DEPENDS ON EXTENSION statement.
+ *
+ * Return value is the address of the altered object.
+ */
+ObjectAddress
+ExecAlterObjectDependsStmt(AlterObjectDependsStmt *stmt)
+{
+	ObjectAddress address;
+	ObjectAddress extAddr;
+	Relation	rel = NULL;
+
+	address = get_object_address_rv(stmt->objectType, stmt->relation, stmt->objname,
+	stmt->objargs, , AccessExclusiveLock, false);
+
+	if (rel)
+		heap_close(rel, NoLock);
+
+	extAddr = get_object_address(OBJECT_EXTENSION, stmt->extname, NULL,
+ , AccessExclusiveLock, false);
+
+	recordDependencyOn(, , DEPENDENCY_AUTO_EXTENSION);
+
+	return address;
+}
+
+/*
  * Executes an ALTER OBJECT / SET SCHEMA statement.  Based on the object
  * type, the function appropriate to that type is executed.
  *
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 6b5d1d6..ecc260d 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -3203,6 +3203,20 @@ _copyRenameStmt(const RenameStmt *from)
 	return newnode;
 }
 
+static AlterObjectDependsStmt *

Re: [HACKERS] [PATCH] Phrase search ported to 9.6

[Alvaro Herrera]

Oleg Bartunov wrote:
> On Thu, Mar 31, 2016 at 9:14 PM, Alvaro Herrera 
> wrote:
> 
> > What led you to choose the ? operator for the FOLLOWED BY semantics?
> > It doesn't seem a terribly natural choice -- most other things seems to
> > use ? as some sort of wildcard.  What about something like "...", so you
> > would do
> >   SELECT q @@ to_tsquery('fatal ... error');
> > and
> >   SELECT q @@ (tsquery 'fatal' ... tsquery 'error');
> >
> >
> originally was $, but then we change it to ?, we don't remember why. During
> warming-up this morning we came to other suggestion
> 
> SELECT q @@ to_tsquery('fatal <> error');
> and
> SELECT q @@ to_tsquery('fatal <2> error');
> 
> How about this ?

Well, I noticed that the docs talk about an operator that can be used in
SQL (outside the tsquery parser), as well as an operator that can be
used inside tsquery.  Inside tsquery anything would be usable, but
outside that it would be good to keep in mind the rest of SQL operators;
and <> means "different from", so using it for FOLLOWED BY seems odd to
me.

My suggestion of ... (ellipsis) is because that's already known as
related to text, used for omissions of words, where the surrounding
words form a phrase.  It seems much more natural to me.

(I also noticed that you can specify a counter together with the
operator, for "at most N words apart", which is not going to work for
the SQL-level operator no matter what you choose.  I suppose that's not
considered a problem)

-- 
Á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] Correction for replication slot creation error message in 9.6

[Ian Barwick]

On 16/04/01 8:15, Michael Paquier wrote:
> On Thu, Mar 31, 2016 at 11:18 PM, Alvaro Herrera
>  wrote:
>> Andres Freund wrote:
>>> On 2016-03-31 10:15:21 +0900, Ian Barwick wrote:
>>
 Patch changes the error message to:

   ERROR:  replication slots can only be used if wal_level is "replica" or 
 "logical"

 Explicitly naming the valid WAL levels matches the wording in the wal_level
 error hint used in a couple of places, i.e.
>>>
>>> The explicit naming makes it much more verbose to change anything around
>>> wal level though, so consider me not a fan of spelling out all levels.
>>
>> I thought we had agreed that we weren't going to consider the wal_level
>> values as a linear scale -- in other words, wordings such as "greater
>> than FOO" are discouraged.  That's always seemed a bit odd to me.
> 
> Yes, that's what I thought as well.

I don't remember if I saw that particular discussion, but same here.
I suppose the alternative would be something like this:

  ERROR: replication slots cannot be used if wal_level is "minimal"

(providing it remains the only "sub-replica" WAL level ;) ).


Regards

Ian Barwick

-- 
 Ian Barwick   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, RemoteDBA, 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] Logical Decoding - Execute join query

[Andres Freund]

Hi,

On 2016-04-01 15:09:59 +0530, hari.prasath wrote:
>   I tried to execute a join query using SPI_execute() in logical
>   decoding part and got inconsistent values (i am referring it as
>   inconsistent since it is returning the old values which is
>   present at the postgresql server start).

You are not allowed to access non catalog tables in an output plugin. To quote 
the manual:
> Read only access to relations is permitted as long as only relations are
> accessed that either have been created by initdb in
> the pg_catalog schema, or have been marked as user
> provided catalog tables using


The reason for that is that we'd have to keep all rows in the tables, if
you wanted to be look at the state "in the past".

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] WIP: Access method extendability

[Aleksander Alekseev]

Hello, Alexander

> Hi!
> 
> New revision of patches is attached.

Code looks much better now, thanks. Still I believe it could be improved.

I don't think that using srand() / rand() in signValue procedure the
way you did is such a good idea. You create a side affect (changing
current randseed) which could cause problems in some cases. And there
is no real need for that. For instance you could use following formula
instead:

hash(attno || hashVal || j)

And a few more things.

> + memset(opaque, 0, sizeof(BloomPageOpaqueData));
> + opaque->maxoff = 0;

This looks a bit redundant.

> + for (my $i = 1; $i <= 10; $i++)

More idiomatic Perl would be `for my $i (1..10)`.

> + UnlockReleaseBuffer(buffer);
> + ReleaseBuffer(metaBuffer);
> + goto away;

In general I don't have anything against goto. But are you sure that
using it here is really justified?

-- 
Best regards,
Aleksander Alekseev
http://eax.me/


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

[HACKERS] Logical Decoding - Execute join query

[hari.prasath]

Hi all,

  

  I tried to execute a join query using SPI_execute() in logical decoding 
part and got inconsistent values (i am referring it as inconsistent since it is 
returning the old values which is present at the postgresql server start).



My data directory has to tables

table1(a integer PRIMARY KEY, b integer, c integer)


table2(x integer PRIMARY KEY, y integer, z integer)




I have table1 as 

 a | b | c 

---+---+---

 1 | 1 | 1

 2 | 2 | 2



and table 2 as



x | y | z 

---+---+---

 1 | 1 | 1

 2 | 2 | 2





Then through psql client inserted a new row to table1 as:  insert into 
table1(3,3,3);



While decoding this insert query i am trying to execute the below query using 
SPI_execute 
SELECT * FROM (table1 LEFT JOIN table2 ON ((table1.a = table2.x)));



And got only 2 rows.

I cant able to get any new rows that are inserted. Are these new values get 
locked somewhere?

 

Is there a way to get inserted changes?




PS:When i restart the pgsql server i can able to get those new values that are 
inserted/updated in previous instance.






cheers

- Harry

Re: [HACKERS] Small patch: --disable-setproctitle flag

[Aleksander Alekseev]

Hello, Andres

> Seems more appropriate to simply manually add a #undef
> HAVE_SETPROCTITLE to pg_config_manual.h in that case. Adding
> configure flags for ephemeral debugger issues seems like a high churn
> activity.

I think you are right. OK.

-- 
Best regards,
Aleksander Alekseev
http://eax.me/


-- 
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-01 10:35:18 +0200, Andres Freund wrote:
> On 2016-04-01 13:50:10 +0530, Dilip Kumar wrote:
> > I think it needs more number of runs.. After seeing this results I did not
> > run head+pinunpin,
> > 
> > Head 64 Client 128 Client
> > -
> > Run1 434860 356945
> > Run2 275815 *275815*
> > Run3 437872 366560
> > Patch 64 Client 128 Client
> > -
> > Run1 429520 372958
> > Run2 446249 *167189*
> > Run3 431066 381592
> > Patch+Pinunpin  64 Client 128 Client
> > --
> > Run1 338298 642535
> > Run2 406240 644187
> > Run3 595439 *285420 *
> 
> Could you describe the exact setup a bit more? Postgres settings,
> pgbench parameters, etc.
> 
> What's the size of BufferDesc after applying the patch?

One interesting thing to do would be to use -P1 during the test and see
how much the performance varies over time.

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] [PATCH] Phrase search ported to 9.6

[Oleg Bartunov]

On Thu, Mar 31, 2016 at 9:14 PM, Alvaro Herrera 
wrote:

> What led you to choose the ? operator for the FOLLOWED BY semantics?
> It doesn't seem a terribly natural choice -- most other things seems to
> use ? as some sort of wildcard.  What about something like "...", so you
> would do
>   SELECT q @@ to_tsquery('fatal ... error');
> and
>   SELECT q @@ (tsquery 'fatal' ... tsquery 'error');
>
>
originally was $, but then we change it to ?, we don't remember why. During
warming-up this morning we came to other suggestion

SELECT q @@ to_tsquery('fatal <> error');
and
SELECT q @@ to_tsquery('fatal <2> error');

How about this ?



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

[Andres Freund]

On 2016-04-01 13:50:10 +0530, Dilip Kumar wrote:
> I think it needs more number of runs.. After seeing this results I did not
> run head+pinunpin,
> 
> Head 64 Client 128 Client
> -
> Run1 434860 356945
> Run2 275815 *275815*
> Run3 437872 366560
> Patch 64 Client 128 Client
> -
> Run1 429520 372958
> Run2 446249 *167189*
> Run3 431066 381592
> Patch+Pinunpin  64 Client 128 Client
> --
> Run1 338298 642535
> Run2 406240 644187
> Run3 595439 *285420 *

Could you describe the exact setup a bit more? Postgres settings,
pgbench parameters, etc.

What's the size of BufferDesc after applying the patch?

Thanks,

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] IF (NOT) EXISTS in psql-completion

[Kyotaro HORIGUCHI]

Hello, sorry for being a bit late.
The attatched are the new version of the patch.. set.

1. 0001-Suggest-IF-NOT-EXISTS-for-tab-completion-of-psql.patch

 Adds IF (NOT) EXISTS completion. It doesn't fix the issue that
 the case of additional keywords don't follow the input.

2. 0002-Make-added-keywords-for-completion-queries-follow-to.patch

 Fixes the case-don't-follow issue by introducing a new macro set
 ADDLISTn(). This leaves the issue for keywords along with
 attributes.

3. 0003-Make-COMPLETE_WITH_ATTR-to-accept-additional-keyword.patch

  Fixes the issue left after 0002 patch. 
  This patch does the following
  things.
  
  1. Change completion_charp from const char * to PQExpBuffer.
  
  2. Chnage COMPLETE_WITH_QUERY and COMPLETE_WITH_ATTR to accept
 an expression instead of string literal.
  
  3. Replace all additional keyword lists in psql_copmletion with
 ADDLISTn() expression.


At Fri, 01 Apr 2016 11:52:03 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI 
 wrote in 
<20160401.115203.98896697.horiguchi.kyot...@lab.ntt.co.jp>
> > > > I found new warning
> > > >
> > > >  tab-complete.c:1438:87: warning: right-hand operand of comma expression
> > > > has no effect [-Wunused-value]
> > >
> > > Mmm. Google said me that gcc 4.9 does so. I'm using 4.8.5 so I
> > > haven't see the warning.
> > >
> > > https://gcc.gnu.org/gcc-4.9/porting_to.html
> > >
> > > 1436:   else if (HeadMatches2("CREATE", "SCHEMA") &&
> > > 1437:SHIFT_TO_LAST1("CREATE") &&
> > > 1438:false) {} /* FALL THROUGH */
...
> > > But the right hand value (true) is actually "used" in the
> > > expression (even though not effective). Perhaps (true && false)
> > > was potimized as false and the true is regarded to be unused?
> > > That's stupid.. Using functions instead of macros seems to solve
> > > this but they needed to be wraped by macros as
> > > additional_kw_query(). That's a pain..
...
> This needs to use gcc 4.9 to address, but CentOS7 doesn't have
> devtools-2 repo so now I'm building CentOS6 environment for this
> purpose. Please wait for a while.

Finally I settled it by replacing comma expression with logical
OR or AND expresssion. gcc 4.9 compains for some unused variables
in flex output but it is the another issue.

I forgot to address COMPLETE_WITH_ATTTR but it needed an overhaul
of some macros and changing the type of completion_charp. The
third patch does it but it might be unacceptable..

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
>From 3c2bbb46749cffc2fd78cdbfdc181128f3c1 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi 
Date: Fri, 5 Feb 2016 16:50:35 +0900
Subject: [PATCH 1/3] Suggest IF (NOT) EXISTS for tab-completion of psql

This patch lets psql to suggest "IF (NOT) EXISTS". Addition to that,
since this patch introduces some mechanism for syntactical robustness,
it allows psql completion to omit some optional part on matching.
---
 src/bin/psql/tab-complete.c | 625 
 1 file changed, 517 insertions(+), 108 deletions(-)

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index eb592bb..a0808cf 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -656,6 +656,10 @@ static const SchemaQuery Query_for_list_of_matviews = {
 "   FROM pg_catalog.pg_roles "\
 "  WHERE substring(pg_catalog.quote_ident(rolname),1,%d)='%s'"
 
+#define Query_for_list_of_rules \
+"SELECT pg_catalog.quote_ident(rulename) FROM pg_catalog.pg_rules "\
+" WHERE substring(pg_catalog.quote_ident(rulename),1,%d)='%s'"
+
 #define Query_for_list_of_grant_roles \
 " SELECT pg_catalog.quote_ident(rolname) "\
 "   FROM pg_catalog.pg_roles "\
@@ -763,6 +767,11 @@ static const SchemaQuery Query_for_list_of_matviews = {
 "SELECT pg_catalog.quote_ident(tmplname) FROM pg_catalog.pg_ts_template "\
 " WHERE substring(pg_catalog.quote_ident(tmplname),1,%d)='%s'"
 
+#define Query_for_list_of_triggers \
+"SELECT pg_catalog.quote_ident(tgname) FROM pg_catalog.pg_trigger "\
+" WHERE substring(pg_catalog.quote_ident(tgname),1,%d)='%s' AND "\
+"   NOT tgisinternal"
+
 #define Query_for_list_of_fdws \
 " SELECT pg_catalog.quote_ident(fdwname) "\
 "   FROM pg_catalog.pg_foreign_data_wrapper "\
@@ -907,7 +916,7 @@ static const pgsql_thing_t words_after_create[] = {
 	{"PARSER", Query_for_list_of_ts_parsers, NULL, THING_NO_SHOW},
 	{"POLICY", NULL, NULL},
 	{"ROLE", Query_for_list_of_roles},
-	{"RULE", "SELECT pg_catalog.quote_ident(rulename) FROM pg_catalog.pg_rules WHERE substring(pg_catalog.quote_ident(rulename),1,%d)='%s'"},
+	{"RULE", Query_for_list_of_rules},
 	{"SCHEMA", Query_for_list_of_schemas},
 	{"SEQUENCE", NULL, _for_list_of_sequences},
 	{"SERVER", Query_for_list_of_servers},
@@ -916,7 +925,7 @@ static const pgsql_thing_t words_after_create[] = {
 	{"TEMP", NULL, NULL, THING_NO_DROP},		/* for CREATE TEMP TABLE ... */
 	{"TEMPLATE", 

Re: [HACKERS] Move PinBuffer and UnpinBuffer to atomics

[Dilip Kumar]

On Thu, Mar 31, 2016 at 5:52 PM, Andres Freund  wrote:

> Here's a WIP patch to evaluate. Dilip/Ashutosh, could you perhaps run
> some benchmarks, to see whether this addresses the performance issues?
>
> I guess it'd both be interesting to compare master with master + patch,
> and this thread's latest patch with the patch additionally applied.
>

I tested it in Power and seen lot of fluctuations in the reading, From this
reading I could not reach to any conclusion.
 only we can say that with (patch + pinunpin), we can reach more than
60.

I think it needs more number of runs.. After seeing this results I did not
run head+pinunpin,

Head 64 Client 128 Client
-
Run1 434860 356945
Run2 275815 *275815*
Run3 437872 366560
Patch 64 Client 128 Client
-
Run1 429520 372958
Run2 446249 *167189*
Run3 431066 381592
Patch+Pinunpin  64 Client 128 Client
--
Run1 338298 642535
Run2 406240 644187
Run3 595439 *285420 *

-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

Re: [HACKERS] Incorrect format in error message

[Andres Freund]

On 2016-04-01 20:18:29 +1300, David Rowley wrote:
> On 1 April 2016 at 17:30, Tom Lane  wrote:
> > David Rowley  writes:
> >> The attached fixes an error message which is incorrectly using an
> >> unsigned format specifier instead of a signed one.
> >
> > Really though, what
> > astonishes me about this example is that we allow indexes at all on
> > system columns other than OID.  None of the other ones can possibly
> > have either a use-case or sensible semantics, can they?  We certainly
> > would not stop to update indexes after changing xmax, for example.
> 
> As for this part. I really don't see how we could disable this without
> breaking pg_restore for database who have such indexes.  My best
> thought is to add some sort of warning during CREATE INDEX, like we do
> for HASH indexes.

As they're currently already not working correctly as indexes, I don't
see throwing an error during pg_restore as being overly harmful.

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] Incorrect format in error message

[David Rowley]

On 1 April 2016 at 17:30, Tom Lane  wrote:
> David Rowley  writes:
>> The attached fixes an error message which is incorrectly using an
>> unsigned format specifier instead of a signed one.
>
> Really though, what
> astonishes me about this example is that we allow indexes at all on
> system columns other than OID.  None of the other ones can possibly
> have either a use-case or sensible semantics, can they?  We certainly
> would not stop to update indexes after changing xmax, for example.

As for this part. I really don't see how we could disable this without
breaking pg_restore for database who have such indexes.  My best
thought is to add some sort of warning during CREATE INDEX, like we do
for HASH indexes.

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


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

Re: [HACKERS] OOM in libpq and infinite loop with getCopyStart()

[Amit Kapila]

On Fri, Apr 1, 2016 at 10:34 AM, Michael Paquier 
wrote:
> On Fri, Apr 1, 2016 at 12:48 PM, Tom Lane  wrote:
> > I thought about this patch a bit more...
> >
> > When I first looked at the patch, I didn't believe that it worked at
> > all: it failed to return a PGRES_COPY_XXX result to the application,
> > and yet nonetheless switched libpq's asyncStatus to PGASYNC_COPY_XXX?
> > Wouldn't things be hopelessly confused?  I tried it out and saw that
> > indeed it seemed to work in psql, and after tracing through that found
> > that psql has no idea what's going on, but when psql issues its next
> > command PQexecStart manages to get us out of the copy state (barring
> > more OOM failures, anyway).  That seems a bit accidental, though,
> > and for sure it wasn't documented in the patch.
>
> From the patch, that's mentioned:
> +* Stop if we are in copy mode and error has occurred, the pending
results
> +* will be discarded during next execution in PQexecStart.
>

Yeah, and same is mentioned in PQexecStart function as well which indicates
that above assumption is right.

>
> > Anyway, the short of my review is that we need more clarity of thought
> > about what state libpq is in after a failure like this, and what that
> > state looks like to the application, and how that should affect how
> > libpq reacts to application calls.
>

Very valid point.  So, if we see with patch, I think libpq will be
in PGASYNC_COPY_XXX state after such a failure and next time when it tries
to again execute statement, it will end copy mode and then allow to proceed
from there.   This design is required for other purposes like silently
discarding left over result.  Now, I think the other option(if possible)
seems to be that we can put libpq in PGASYNC_IDLE, if we get such an error
as we do at some other places in case of error as below and return OOM
failure as we do in patch.

PQgetResult()

{

..

if (flushResult ||

pqWait(TRUE, FALSE, conn) ||

pqReadData(conn) < 0)

{

/*

* conn->errorMessage has been set by pqWait or pqReadData. We

* want to append it to any already-received error message.

*/

pqSaveErrorResult(conn);

conn->asyncStatus = PGASYNC_IDLE;

return pqPrepareAsyncResult(conn);

}
..
}

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

Re: [HACKERS] Speedup twophase transactions

[Michael Paquier]

On Wed, Mar 30, 2016 at 10:19 PM, Stas Kelvich  wrote:
> Hm, it’s hard to create descriptive names because test changes master/slave
> roles for that nodes several times during test.

Really? the names used in the patch help less then.

> It’s possible to call them
> “node1” and “node2” but I’m not sure that improves something. But anyway I’m
> not insisting on that particular names and will agree with any reasonable
> suggestion.

I would suggest the following name modifications, node names have been
introduced to help tracking of each node's log:
- Candie => master
- Django => slave or just standby
There is no need for complication! And each node's log file is
prefixed by the test number. Note that in other tests there are no
promotions, or fallbacks done, but we stick with a node name that
represents the initial state of the cluster.

> +# Switch to synchronous replication
> +$node_master->append_conf('postgresql.conf', qq(
> +synchronous_standby_names = '*'
> +));
> +$node_master->restart;
> Reloading would be fine.
>
> Good catch, done.

+$node_master->psql('postgres', "select pg_reload_conf()");

It would be cleaner to introduce a new routine in PostgresNode that
calls pg_ctl reload (mentioned in the N-sync patch as well, that would
be useful for many purposes).

> All accesses to 2pc dummies in ProcArray are covered with TwoPhaseStateLock,
> so I thick that’s safe. Also I’ve deleted comment above that block, probably
> it’s more confusing than descriptive.

OK, you removed the use to allProcs. Though by reading again the code
just holding TwoPhaseStateLock that's actually fine.

The patch needs a small cleanup:
$ git diff master --check
src/test/recovery/t/006_twophase.pl:224: new blank line at EOF.

006_twophase.pl should be renamed to 007. It keeps its license to
kill, and gains in being famous.

- * Scan the pg_twophase directory and setup all the required information to
- * allow standby queries to treat prepared transactions as still active.
- * This is never called at the end of recovery - we use
- * RecoverPreparedTransactions() at that point.
+ * It's a caller responsibility to call MarkAsPrepared() on returned gxact.
  *
Wouldn't it be more simple to just call MarkAsPrepared at the end of
RecoverPreparedFromBuffer?

While testing the patch, I found a bug in the recovery conflict code
path. You can do the following to reproduce it:
1) Start a master with a standby
2) prepare a transaction on master
3) Stop immediate on standby to force replay
4) commit prepare transaction on master
5) When starting the standby, it remains stuck here:
* thread #1: tid = 0x229b4, 0x7fff8e2d4f96
libsystem_kernel.dylib`poll + 10, queue = 'com.apple.main-thread',
stop reason = signal SIGSTOP
  * frame #0: 0x7fff8e2d4f96 libsystem_kernel.dylib`poll + 10
frame #1: 0x000107e5e043
postgres`WaitEventSetWaitBlock(set=0x7f90c20596a8, cur_timeout=-1,
occurred_events=0x7fff581efd28, nevents=1) + 51 at latch.c:1102
frame #2: 0x000107e5da26
postgres`WaitEventSetWait(set=0x7f90c20596a8, timeout=-1,
occurred_events=0x7fff581efd28, nevents=1) + 390 at latch.c:935
frame #3: 0x000107e5d4c7
postgres`WaitLatchOrSocket(latch=0x000111432464, wakeEvents=1,
sock=-1, timeout=-1) + 343 at latch.c:347
frame #4: 0x000107e5d36a
postgres`WaitLatch(latch=0x000111432464, wakeEvents=1, timeout=0)
+ 42 at latch.c:302
frame #5: 0x000107e7b5a6 postgres`ProcWaitForSignal + 38 at proc.c:1731
frame #6: 0x000107e6a4eb
postgres`ResolveRecoveryConflictWithLock(locktag=LOCKTAG at
0x7fff581efde8) + 187 at standby.c:391
frame #7: 0x000107e7a6a8
postgres`ProcSleep(locallock=0x7f90c203dac8,
lockMethodTable=0x0001082f6218) + 1128 at proc.c:1215
frame #8: 0x000107e72886
postgres`WaitOnLock(locallock=0x7f90c203dac8,
owner=0x) + 358 at lock.c:1703
frame #9: 0x000107e70f93
postgres`LockAcquireExtended(locktag=0x7fff581f0238, lockmode=8,
sessionLock='\x01', dontWait='\0', reportMemoryError='\0') + 2819 at
lock.c:998
frame #10: 0x000107e6a9a6
postgres`StandbyAcquireAccessExclusiveLock(xid=863, dbOid=16384,
relOid=16385) + 358 at standby.c:627
frame #11: 0x000107e6af0b
postgres`standby_redo(record=0x7f90c2041e38) + 251 at
standby.c:809
frame #12: 0x000107b0e227 postgres`StartupXLOG + 9351 at xlog.c:6871
It seems that the replay on on-memory state of the PREPARE transaction
is conflicting directly with replay.
-- 
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] Incorrect format in error message

[David Rowley]

On 1 April 2016 at 17:30, Tom Lane  wrote:
> David Rowley  writes:
>> The attached fixes an error message which is incorrectly using an
>> unsigned format specifier instead of a signed one.
>
> I think that's the tip of the iceberg :-(.  For starters, the code
> allows ObjectIdAttributeNumber without regard for the fact that the
> next line will dump core on a negative attno.  Really though, what
> astonishes me about this example is that we allow indexes at all on
> system columns other than OID.  None of the other ones can possibly
> have either a use-case or sensible semantics, can they?  We certainly
> would not stop to update indexes after changing xmax, for example.

ouch. Yeah that's not going to work very well. I guess nobody's tried
that with a unique index on an OID column yet then.

I've changed the patch around a little to fix the crash. I was a bit
worried as logical decoding obviously has not yet been tested with an
OID unique index as the replica indentity, so I gave it a quick test
to try to give myself a little piece of mind that this won't uncover
something else;

# create table t (a int) with oids;
CREATE TABLE
# create unique index t_oid_idx on t(oid);
CREATE INDEX
# alter table t replica identity using index t_oid_idx;
ALTER TABLE
# insert into t values(123);
INSERT 24593 1
# delete from t;

On the receive side:

BEGIN 606
table public.t: INSERT: oid[oid]:24593 a[integer]:123
COMMIT 606
BEGIN 607
table public.t: DELETE: oid[oid]:24593
COMMIT 607

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


replica_identity_crash_fix.patch
Description: Binary data

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

Re: [HACKERS] Timeline following for logical slots

[Andres Freund]

Hi,

On 2016-04-01 08:46:01 +0200, Andres Freund wrote:
> That's a fundamental misunderstanding on your part (perhaps created by
> imprecise docs).

> > Speaking of which, did you see the proposed README I sent for
> > src/backend/replication/logical ?
> 
> I skimmed it. But given we have a CF full of patches, some submitted
> over a year ago, it seems unfair to spend time on a patch submitted a
> few days ago.

For that purpos

WRT design readme, it might be interesting to look at 0009 in
http://archives.postgresql.org/message-id/20140127162006.GA25670%40awork2.anarazel.de

That's not up2date obviously, but it still might help.

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] Timeline following for logical slots

[Andres Freund]

Hi,

On 2016-04-01 13:16:18 +0800, Craig Ringer wrote:
> I think it's pretty unsafe from SQL, to be sure.
> 
> Unless failover slots get in to 9.6 we'll need to do exactly that from
> internal C stuff in pglogical to support following physical failover,

I know. And this makes me scared shitless.


I'll refuse to debug anything related to decoding, when this "feature"
has been used. And I think there'll be plenty; if you notice the issues
that is.


The more I think about it, the more I think we should rip this out
again, until we have the actual feature is in. With proper review.


> However: It's safe for the slot state on the replica to be somewhat behind
> the local replay from the master, so the slot state on the replica is older
> than what it would've been at an equivalent time on the master... so long
> as it's not so far behind that the replica has replayed vacuum activity
> that removes rows still required by the slot's declared catalog_xmin. Even
> then it should actually be OK in practice because the failing-over client
> will always have replayed past that point on the master (otherwise
> catalog_xmin couldn't have advanced on the master), so it'll always ask to
> start replay at a point at or after the lsn where the catalog_xmin was
> advanced to its most recent value on the old master before failover. It's
> only safe for walsender based decoding though, since there's no way to
> specify the startpoint for sql-level decoding.

This whole logic fails entirely flats on its face by the fact that even
if you specify a startpoint, we read older WAL and process the
records. The startpoint filters every transaction that commits earlier
than the "startpoint". But with a long-running transaction you still
will have old changes being processed, which need the corresponding
catalog state.


> My only real worry there is whether invalidations are a concern - if
> internal replay starts at the restart_lsn and replays over part of history
> where the catalog entries have been vacuumed before it starts collecting
> rows for return to the decoding client at the requested decoding
> startpoint, can that cause problems with invalidation processing?

Yes.


> It's also OK for the slot state to be *ahead* of the replica's replay
> position, i.e. from the future. restart_lsn and catalog_xmin will be higher
> than they were on the master at the same point in time, but that doesn't
> matter at all, since the client will always be requesting to start from
> that point or later, having replayed up to there on the old master before
> failover.

I don't think that's true. See my point above about startpoint.



> > Andres, I tried to address your comments as best I could. The main one
> > that
> > > I think stayed open was about the loop that finds the last timeline on a
> > > segment. If you think that's better done by directly scanning the List*
> > of
> > > timeline history entries I'm happy to prep a follow-up.
> >
> > Have to look again.
> >
> > +* We start reading xlog from the restart lsn, even though in
> > +* CreateDecodingContext we set the snapshot builder up using the
> > +* slot's confirmed_flush. This means we might read xlog we don't
> > +* actually decode rows from, but the snapshot builder might need
> > it
> > +* to get to a consistent point. The point we start returning data
> > to
> > +* *users* at is the confirmed_flush lsn set up in the decoding
> > +* context.
> > +*/
> > still seems pretty misleading - and pretty much unrelated to the
> > callsite.
> >
> 
> 
> I think it's pretty confusing that the function uses's the slot's
> restart_lsn and never refers to confirmed_flush which is where it actually
> starts decoding from as far as the user's concerned. It took me a while to
> figure out what was going on there.

That's a fundamental misunderstanding on your part (perhaps created by
imprecise docs). The startpoint isn't about where to start
decoding. It's about skip calling the output plugin of a transaction if
it *commits* before the startpoint. We almost always will *decode* rows
from before the startpoint.  And that's pretty much unavoidable: The
consumer of a decoding slot only really can do anything with commit LSNs
wrt replay progress: But for a remembered progress LSN there can be a
lot of in-progress xacts (about which said client doesn't know
anything); and they start *earlier* than the commit LSN of the just
replayed xact. So we have to be able to re-process their state and send
it out.


> I think the comment would be less necessary, and could be moved up to the
> CreateDecodingContext call, if we passed the slot's confirmed_flush
> explicitly to CreateDecodingContext instead of passing InvalidXLogRecPtr
> and having the CreateDecodingContext call infer the start point. That way
> what's going on would be a lot clearer when reading the code.

How would that solve anything? We still need to process the old records,