Re: [HACKERS] UPDATE of partition key

2017-05-28 Thread Amit Khandekar
On 24 May 2017 at 20:16, Amit Kapila  wrote:
> On Wed, May 24, 2017 at 8:14 PM, Amit Kapila  wrote:
>> Apart from above, there is one open issue [1]
>>
>
> Forget to mention the link, doing it now.
>
> [1] - 
> https://www.postgresql.org/message-id/CAA4eK1KEZQ%2BCyXbBzfn1jFHoEfa_OemDLhLyy7xfD1QUZLo1DQ%40mail.gmail.com

I am not sure right now whether making the t_ctid of such tuples to
Invalid would be a right option, especially because I think there can
be already some other meaning if t_ctid is not valid. But may be we
can check this more.

If we decide to error out using some way, I would be inclined towards
considering re-using some combinations of infomask bits (like
HEAP_MOVED_OFF as suggested upthread) rather than using invalid t_ctid
value.

But I think, we can also take step-by-step approach even for v11. If
we agree that it is ok to silently do the updates as long as we
document the behaviour, we can go ahead and do this, and then as a
second step, implement error handling as a separate patch. If that
patch does not materialize, we at least have the current behaviour
documented.

Ideally, I think we would have liked if we were somehow able to make
the row-movement UPDATE itself abort if it finds any normal
updates waiting for it to finish, rather than making the normal
updates fail because a row-movement occurred . But I think we will
have to live with it.


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


Re: [HACKERS] logical replication - still unstable after all these months

2017-05-28 Thread Mark Kirkwood



On 29/05/17 13:33, Jeff Janes wrote:
On Sun, May 28, 2017 at 3:17 PM, Mark Kirkwood 
> 
wrote:


On 28/05/17 19:01, Mark Kirkwood wrote:


So running in cloud land now...so for no errors - will update.




The framework ran 600 tests last night, and I see 3 'NOK' results,
i.e 3 failed test runs (all scale 25 and 8 pgbench clients). Given
the way the test decides on failure (gets tired of waiting for the
table md5's to match) - it begs the question 'What if it had
waited a bit longer'? However from what I can see in all cases:

- the rowcounts were the same in master and replica
- the md5 of pgbench_accounts was different


All four tables should be wrong if there is still a transaction it is 
waiting for, as all the changes happen in a single transaction.




Yeah, my thought exactly.

I also got a failure, after 87 iterations of a similar test case.  It 
waited for hours, as mine requires manual intervention to stop 
waiting.  On the subscriber, one account still had a zero balance, 
while the history table on the subscriber agreed with both history and 
accounts on the publisher and the account should not have been zero, 
so definitely a transaction atomicity got busted.




Interesting, great that we are seeing the same thing.

I altered the script to also save the tellers and branches tables and 
repeated the runs, but so far it hasn't failed again in over 800 
iterations using the altered script.



...so does seem possible that there is some bug being tickled
here. Unfortunately the test framework blasts away the failed
tables and subscription and continues on...I'm going to amend it
to stop on failure so I can have a closer look at what happened.


What would you want to look at?  Would saving the WAL from the master 
be helpful?





Good question - I initially wanted to see if anything changed if I 
waited longer (which you have already figured out) and what was actually 
wrong with the accounts record (which you have gotten to as well)! Nice. 
After that, I'd thought of doing another transaction on an accounts 
record that lives in the same page as the 'gammy' one on the master - 
generally poke about and see what is happening :-)


regards

Mark


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


[HACKERS] psql: Activate pager only for height, not width

2017-05-28 Thread Brendan Jurd
Hello hackers,

I am often frustrated by the default behaviour of the psql pager, which
will activate a pager if the output is deemed to be "too wide" for the
terminal, regardless of the number of lines output, and of the
pager_min_lines setting.

This behaviour is sometimes desirable, but in my use patterns it is more
often the case that I want the pager to activate for output longer than
terminal height, whereas for output a little wider than the terminal, I am
happy for there to be some wrapping.  This is especially the case with "\d"
output for tables, where, at 80 columns, very often the only wrapping is in
the table borders and constraint/trigger definitions.

Usually I turn the pager off completely, and only switch it on when I am
about to execute something that will return many rows, but what I'd really
like is some way to tell psql to activate the pager as normal for height,
but to ignore width.  My first thought was an alternate mode to \pset pager
-- to {'on' | 'off' | 'always'} we could add 'height'.

Another option is to add the ability to specify the number of columns which
psql considers "too wide", analogous to pager_min_lines.  I could then set
pager_min_cols to something around 150 which would work nicely for my
situation.

I don't have strong opinions about how the options are constructed, as long
as it is possible to obtain the behaviour.

I would be happy to produce a patch, if this seems like an acceptable
feature add.

Regards,
BJ


Re: [HACKERS] logical replication - still unstable after all these months

2017-05-28 Thread Mark Kirkwood

On 29/05/17 16:26, Erik Rijkers wrote:


On 2017-05-29 00:17, Mark Kirkwood wrote:

On 28/05/17 19:01, Mark Kirkwood wrote:


So running in cloud land now...so for no errors - will update.


The framework ran 600 tests last night, and I see 3 'NOK' results, i.e
3 failed test runs (all scale 25 and 8 pgbench clients). Given the way


Could you also give the params for the successful runs?


Scale 25, clients 90 and 64, scale 5 clients 90, 64 and 8 (i.e the 
defaults in the driver script).


Can you say anything about hardware?  (My experience is that older, 
slower, 'worse' hardware makes for more fails.)


It's running in a openstack cloud (so is a libvirt guest): 4 cpus, 4 GB 
ram and 2 disks: one for each Postgres instance, both formatted xfs. Hmm 
so maybe I should run a VM on my workstation and crank the IOPS limit 
way down...in the meantime I'll just let it run :-)



Many thanks, by the way.  I'm glad that it turns out I'm probably not 
doing something uniquely stupid (although I'm not glad that there 
seems to be a bug, and an elusive one at that)





Yeah looks like something subtle :-( Hopefully now its out in the open 
we'll all figure it together!


regards

Mark


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


Re: [HACKERS] logical replication - still unstable after all these months

2017-05-28 Thread Erik Rijkers

On 2017-05-29 03:33, Jeff Janes wrote:

On Sun, May 28, 2017 at 3:17 PM, Mark Kirkwood <
mark.kirkw...@catalyst.net.nz> wrote:

I also got a failure, after 87 iterations of a similar test case.  It

[...]
repeated the runs, but so far it hasn't failed again in over 800 
iterations


Could you give the params for the successful runs?
(ideally, a  grep | sort | uniq -c  of the ran  pgbench lines )


Can you say anything about hardware?


Thanks for repeating my lengthy tests.


Erik Rijkers


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


Re: [HACKERS] logical replication - still unstable after all these months

2017-05-28 Thread Erik Rijkers

On 2017-05-29 00:17, Mark Kirkwood wrote:

On 28/05/17 19:01, Mark Kirkwood wrote:


So running in cloud land now...so for no errors - will update.


The framework ran 600 tests last night, and I see 3 'NOK' results, i.e
3 failed test runs (all scale 25 and 8 pgbench clients). Given the way


Could you also give the params for the successful runs?

Can you say anything about hardware?  (My experience is that older, 
slower, 'worse' hardware makes for more fails.)



Many thanks, by the way.  I'm glad that it turns out I'm probably not 
doing something uniquely stupid (although I'm not glad that there seems 
to be a bug, and an elusive one at that)



Erik Rijkers


--
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] Extra Vietnamese unaccent rules

2017-05-28 Thread Thomas Munro
On Sun, May 28, 2017 at 7:55 PM, Dang Minh Huong  wrote:
> [Quoting Thomas]
>> You don't have to worry about decoding that line, it's all done in
>> that Python script.  The problem is just in the function
>> is_letter_with_marks().  Instead of just checking if combining_ids[0]
>> is a plain letter, it looks like it should also check if
>> combining_ids[0] itself is a letter with marks.  Also get_plain_letter
>> would need to be able to recurse to extract the "a".
>
> Thanks for reporting and lecture about unicode.
> I attached a patch as the instruction from Thomas. Could you confirm it.

-   is_plain_letter(table[codepoint.combining_ids[0]]) and \
+   (is_plain_letter(table[codepoint.combining_ids[0]]) or\
+len(table[codepoint.combining_ids[0]].combining_ids) > 1) and \

Shouldn't you use "or is_letter_with_marks()", instead of "or len(...)
> 1"?  Your test might catch something that isn't based on a 'letter'
(according to is_plain_letter).  Otherwise this looks pretty good to
me.  Please add it to the next commitfest.

I expect that some users in Vietnam will consider this to be a bugfix,
which raises the question of whether to backpatch it.  Perhaps we
could consider fixing it for 10.  Then users of older versions could
grab the rules file from 10 to use with 9.whatever if they want to do
that and reindex their data as appropriate.

> [Quoting Michael]
>> Actually, with the recent work that has been done with
>> unicode_norm_table.h which has been to transpose UnicodeData.txt into
>> user-friendly tables, shouldn't the python script of unaccent/ be
>> replaced by something that works on this table? This does a canonical
>> decomposition but just keeps the first characters with a class
>> ordering of 0. So we have basic APIs able to look at UnicodeData.txt
>> and let caller do decision making with the result returned.
>
> Thanks, i will learning about it.

It seems like that could be useful for runtime use (I'm sure there is
a whole world of Unicode support we could add), but here we're only
trying to generate a mapping file to add to the source tree, so I'm
not sure how it's relevant.

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


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


Re: [HACKERS] logical replication - still unstable after all these months

2017-05-28 Thread Jeff Janes
On Sun, May 28, 2017 at 3:17 PM, Mark Kirkwood <
mark.kirkw...@catalyst.net.nz> wrote:

> On 28/05/17 19:01, Mark Kirkwood wrote:
>
>
>> So running in cloud land now...so for no errors - will update.
>>
>>
>>
>>
> The framework ran 600 tests last night, and I see 3 'NOK' results, i.e 3
> failed test runs (all scale 25 and 8 pgbench clients). Given the way the
> test decides on failure (gets tired of waiting for the table md5's to
> match) - it begs the question 'What if it had waited a bit longer'? However
> from what I can see in all cases:
>
> - the rowcounts were the same in master and replica
> - the md5 of pgbench_accounts was different
>

All four tables should be wrong if there is still a transaction it is
waiting for, as all the changes happen in a single transaction.

I also got a failure, after 87 iterations of a similar test case.  It
waited for hours, as mine requires manual intervention to stop waiting.  On
the subscriber, one account still had a zero balance, while the history
table on the subscriber agreed with both history and accounts on the
publisher and the account should not have been zero, so definitely a
transaction atomicity got busted.

I altered the script to also save the tellers and branches tables and
repeated the runs, but so far it hasn't failed again in over 800 iterations
using the altered script.


>
> ...so does seem possible that there is some bug being tickled here.
> Unfortunately the test framework blasts away the failed tables and
> subscription and continues on...I'm going to amend it to stop on failure so
> I can have a closer look at what happened.
>

What would you want to look at?  Would saving the WAL from the master be
helpful?

Cheers,

Jeff


Re: [HACKERS] logical replication - still unstable after all these months

2017-05-28 Thread Mark Kirkwood

On 28/05/17 19:01, Mark Kirkwood wrote:



So running in cloud land now...so for no errors - will update.





The framework ran 600 tests last night, and I see 3 'NOK' results, i.e 3 
failed test runs (all scale 25 and 8 pgbench clients). Given the way the 
test decides on failure (gets tired of waiting for the table md5's to 
match) - it begs the question 'What if it had waited a bit longer'? 
However from what I can see in all cases:


- the rowcounts were the same in master and replica
- the md5 of pgbench_accounts was different

...so does seem possible that there is some bug being tickled here. 
Unfortunately the test framework blasts away the failed tables and 
subscription and continues on...I'm going to amend it to stop on failure 
so I can have a closer look at what happened.


regards

Mark


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


[HACKERS] Inconsistent syntax for NumericOnly grammar production

2017-05-28 Thread Tom Lane
I noticed that gram.y has

NumericOnly:
FCONST  { $$ = makeFloat($1); }
| '-' FCONST
{
$$ = makeFloat($2);
doNegateFloat($$);
}
| SignedIconst  { $$ = makeInteger($1); }
;

but

SignedIconst: Iconst{ $$ = $1; }
| '+' Iconst{ $$ = + $2; }
| '-' Iconst{ $$ = - $2; }
;

The inconsistency here means that you can do, for example,

regression=# set random_page_cost = +4;
SET
regression=# set random_page_cost = 4.2;
SET

but not

regression=# set random_page_cost = +4.2;
ERROR:  syntax error at or near "4.2"
LINE 1: set random_page_cost = +4.2;
^

That's weird enough in itself, and the problem is about to get more
widespread because the partbound_datum production depends on NumericOnly.

Any objections to allowing "+ FCONST" here?  I'm inclined to
fix this and back-patch it as well.

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] Index created in BEFORE trigger not updated during INSERT

2017-05-28 Thread Tom Lane
Andres Freund  writes:
> On 2017-05-24 08:26:24 -0400, Robert Haas wrote:
>> I'm willing to bet that nobody ever thought about that case very hard.
>> It seems like we should either make it work or prohibit it, but I
>> can't actually see quite how to do either off-hand.

> Hm, strategically sprinkled CheckTableNotInUse() might do the trick?

+1.  We can't reasonably make it work: the outer query already has its
list of indexes that need to be inserted into.  Also, if you try to
make the index via ALTER TABLE ADD CONSTRAINT in the trigger, that will
give you "cannot ALTER TABLE "mytable" because it is being used by active
queries in this session" because of the check in AlterTable().

It doesn't seem terribly hard to fix the CREATE INDEX case to behave
likewise, but I wonder how many other cases we've missed?

One small issue is that naive use of CheckTableNotInUse would produce an
error reading "cannot CREATE INDEX "mytable" because ...", which is not
exactly on point.  It'd be better for it to say "cannot create an index on
"mytable" because ...".  However, given that it took twenty years for
anybody to notice this, maybe it's close enough.

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] Allow GiST opcalsses without compress\decompres functions

2017-05-28 Thread Tom Lane
Andrew Borodin  writes:
> I'm not expert in toasting, cube's compress does nothing while cube
> decomress does detoasting. Is this for reason?

The original input datum could be toasted --- at least to the extent of
being compressed in-line or having short header; I do not think we allow
out-of-line storage in an index.  This is OK for storage within the index,
and it would also be OK for an IOS to return the value in that form.
But the opclass's consistent function might expect to be always given an
untoasted input, in which case you'd need the decompress function to fix
that up.

Mind you, I'm not saying that this would represent good design;
datatype-specific functions that expect somebody else to have
detoasted their input seem pretty fragile.  But it might be how
cube's GIST support works.

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] Allow GiST opcalsses without compress\decompres functions

2017-05-28 Thread Andrew Borodin
28 мая 2017 г. 11:15 PM пользователь "Tom Lane"  написал:

Andrew Borodin  writes:
> 2017-05-28 22:22 GMT+05:00 Tom Lane :
>> 2. If compress/decompress are omitted, then we could support index-only
>> scans all the time, that is a no-op fetch function would work.  The
>> patch should cover that interaction too.

> I do not think so. Decompress have to get att to the state where
> consistency function can understand it. In theory. I've checked like a
> dozen of types and have not found places where fetch should differ
> from decompress.

But if compress is omitted, then we know the in-index representation
is the same as the original.  Therefore the fetch function would always
be a no-op and we can implement IOS whether or not the opclass designer
bothered to supply one.

regards, tom lane

True. If there is no code to "hash" original value, it is not hashed, it's
whole...

I'm not expert in toasting, cube's compress does nothing while cube
decomress does detoasting. Is this for reason?

Best regards, Andrey Borodin.


Re: [HACKERS] [pgsql-translators] pg_upgrade translation

2017-05-28 Thread Alvaro Herrera
Alvaro Herrera wrote:
> Translatability was turned on for pg_upgrade on pg10, but it needs some
> work.  The choices are we fix it now, or we revert the addition of NLS
> there and we do it for PG11.

Here are two patches to fix the messages, replacing "new" with "target"
and "old" with "source".  I was going to do just the first one
initially, but I figured that the output would become wildly
inconsistent if I stopped at that point.

Now we still have one inconsistency left: the command-line options
--old-bindir --new-datadir etc all contain the words "new" and "old".
Same with a bunch of environment variables.  I think the real solution
is that we should change those to --source-bindir and --target-datadir
respectively (and the other ones continue to work for backwards
compatibility), but then maybe leave them alone.

I'll continue fixing things.

-- 
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From 87b904eeaf82dbd402d5f764de8cd2b209cd17ba Mon Sep 17 00:00:00 2001
From: Alvaro Herrera 
Date: Sat, 27 May 2017 22:24:24 -0400
Subject: [PATCH 1/2] pg_upgrade: Fix "%s server/cluster" wording

---
 src/bin/pg_upgrade/check.c   | 16 
 src/bin/pg_upgrade/controldata.c |  9 ++---
 src/bin/pg_upgrade/info.c|  6 +-
 src/bin/pg_upgrade/option.c  |  6 --
 src/bin/pg_upgrade/pg_upgrade.h  |  2 --
 src/bin/pg_upgrade/server.c  | 19 ++-
 6 files changed, 41 insertions(+), 17 deletions(-)

diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index 8b9e81eb40..9df4270b3c 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -768,8 +768,12 @@ check_for_prepared_transactions(ClusterInfo *cluster)
"FROM 
pg_catalog.pg_prepared_xacts");
 
if (PQntuples(res) != 0)
-   pg_fatal("The %s cluster contains prepared transactions\n",
-CLUSTER_NAME(cluster));
+   {
+   if (cluster == _cluster)
+   pg_fatal("The source cluster contains prepared 
transactions\n");
+   else
+   pg_fatal("The target cluster contains prepared 
transactions\n");
+   }
 
PQclear(res);
 
@@ -1080,8 +1084,12 @@ check_for_pg_role_prefix(ClusterInfo *cluster)
"WHERE rolname ~ 
'^pg_'");
 
if (PQntuples(res) != 0)
-   pg_fatal("The %s cluster contains roles starting with 'pg_'\n",
-CLUSTER_NAME(cluster));
+   {
+   if (cluster == _cluster)
+   pg_fatal("The source cluster contains roles starting 
with 'pg_'\n");
+   else
+   pg_fatal("The target cluster contains roles starting 
with 'pg_'\n");
+   }
 
PQclear(res);
 
diff --git a/src/bin/pg_upgrade/controldata.c b/src/bin/pg_upgrade/controldata.c
index 519256851c..abebb15a94 100644
--- a/src/bin/pg_upgrade/controldata.c
+++ b/src/bin/pg_upgrade/controldata.c
@@ -474,9 +474,12 @@ get_control_data(ClusterInfo *cluster, bool live_check)
 cluster->controldata.ctrl_ver >= 
LARGE_OBJECT_SIZE_PG_CONTROL_VER) ||
!got_date_is_int || !got_data_checksum_version)
{
-   pg_log(PG_REPORT,
-  "The %s cluster lacks some required control 
information:\n",
-  CLUSTER_NAME(cluster));
+   if (cluster == _cluster)
+   pg_log(PG_REPORT,
+  "The source cluster lacks some required 
control information:\n");
+   else
+   pg_log(PG_REPORT,
+  "The target cluster lacks some required 
control information:\n");
 
if (!got_xid)
pg_log(PG_REPORT, "  checkpoint next XID\n");
diff --git a/src/bin/pg_upgrade/info.c b/src/bin/pg_upgrade/info.c
index ab6328eef5..669524c74a 100644
--- a/src/bin/pg_upgrade/info.c
+++ b/src/bin/pg_upgrade/info.c
@@ -320,7 +320,11 @@ get_db_and_rel_infos(ClusterInfo *cluster)
for (dbnum = 0; dbnum < cluster->dbarr.ndbs; dbnum++)
get_rel_infos(cluster, >dbarr.dbs[dbnum]);
 
-   pg_log(PG_VERBOSE, "\n%s databases:\n", CLUSTER_NAME(cluster));
+   if (cluster == _cluster)
+   pg_log(PG_VERBOSE, "\nsource databases:\n");
+   else
+   pg_log(PG_VERBOSE, "\ntarget databases:\n");
+
if (log_opts.verbose)
print_db_infos(>dbarr);
 }
diff --git a/src/bin/pg_upgrade/option.c b/src/bin/pg_upgrade/option.c
index 5a556e7b30..1406c5b46f 100644
--- a/src/bin/pg_upgrade/option.c
+++ b/src/bin/pg_upgrade/option.c
@@ -405,8 +405,10 @@ adjust_data_dir(ClusterInfo *cluster)
 
/* Must be a configuration 

Re: [HACKERS] Allow GiST opcalsses without compress\decompres functions

2017-05-28 Thread Tom Lane
Andrew Borodin  writes:
> 2017-05-28 22:22 GMT+05:00 Tom Lane :
>> 2. If compress/decompress are omitted, then we could support index-only
>> scans all the time, that is a no-op fetch function would work.  The
>> patch should cover that interaction too.

> I do not think so. Decompress have to get att to the state where
> consistency function can understand it. In theory. I've checked like a
> dozen of types and have not found places where fetch should differ
> from decompress.

But if compress is omitted, then we know the in-index representation
is the same as the original.  Therefore the fetch function would always
be a no-op and we can implement IOS whether or not the opclass designer
bothered to supply one.

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] Allow GiST opcalsses without compress\decompres functions

2017-05-28 Thread Andrew Borodin
Thanks, Tom!

2017-05-28 22:22 GMT+05:00 Tom Lane :
> Andrew Borodin  writes:
>> Maybe we should make compress\decompress functions optional?
>
> 1. You'll need to adjust the documentation (gist.sgml) not just the code.
Sure, I'll check out where compression is mentioned and update docs.

> 2. If compress/decompress are omitted, then we could support index-only
> scans all the time, that is a no-op fetch function would work.  The
> patch should cover that interaction too.
I do not think so. Decompress have to get att to the state where
consistency function can understand it. In theory. I've checked like a
dozen of types and have not found places where fetch should differ
from decompress.

> 3. Personally I wouldn't bother with the separate compressed[] flags,
> just look at OidIsValid(giststate->compressFn[i].fn_oid).
That would save few bytes, but make compress\decompress code slightly
harder to read. Though some comments will help.

> 4. I don't think you thought very carefully about the combinations
> where only one of the two functions is supplied.  I can definitely
> see that compress + no decompress could be sensible.  Less sure
> about the other case, but why not allow it?  We could just say that
> an omitted function is taken to represent a no-op transformation.
Well, I thought only about the exclusion of this situation(when only
one function is supplied). But, Indeed, I've seen many opclasses where
compress was doing a work (like simplifying the data) while decompress
is just NOP.
I'll think more about it.
decompress-only opclass seems like having zero sense in adjusting
tuple on internal page. We decompress att, update it, then store it
back. Then, once upon a time, decompress it again. Looks odd. But this
does not look like a place where opcalss developer can introduce new
bugs, so I do not see reasons to forbid having only decompress
function.

> Please add this to the next commitfest.

OK.
Thank you for your comments. I'll address them and put a patch to the
commitfest.

Best regards, Andrey Borodin.


-- 
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] PostgreSQL 10 changes in exclusion constraints - did something change? CASE WHEN behavior oddity

2017-05-28 Thread Tom Lane
"Regina Obe"  writes:
> Is this behavior going to stay or change?
> It seems inconsistent from a user perspective that
> CASE constant  == short-circuit skipping over SRFs that may  otherwise
> fail
> While 
> CASE not_constant_table_dependent  doesn't short-circuit.
> I can understand the motive behind it, it just feels a little inconsistent
> from an end-user POV.

After thinking about this for awhile, I agree that what we've got now
isn't too satisfactory.  We can make an analogy between SRFs and
aggregate functions: both of them look like simple function calls
syntactically, but they have global effects on the semantics of the
query, particularly on how many rows are returned.

In the case of aggregates, there is long-standing precedent that we
can optimize away individual aggregate calls but the query semantics
do not change, ie you get one output row (or one per GROUP BY group)
even if every last aggregate call disappears due to CASE simplification.
The same was true for deletion of SRFs by CASE-simplification before
v10, but now we've broken that, which seems like a clear bug.

I think it would be possible to teach eval_const_expressions that
it must not discard CASE/COALESCE subexpressions that contain SRFs,
which would preserve the rule that expression simplification doesn't
change the query semantics.

Another possibility is to say that we've broken this situation
irretrievably and we should start throwing errors for SRFs in
places where they'd be conditionally evaluated.  That's not real
nice perhaps, but it's better than the way things are right now.

Thoughts?

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] execGrouping.c limit on work_mem

2017-05-28 Thread Tom Lane
Jeff Janes  writes:
> In BuildTupleHashTable
> /* Limit initial table size request to not more than work_mem */
> nbuckets = Min(nbuckets, (long) ((work_mem * 1024L) / entrysize));

> Is this a good idea?  If the caller of this code has no respect for
> work_mem, they are still going to blow it out of the water.  Now we will
> just do a bunch of hash-table splitting in the process.  That is only going
> to add to the pain.

It looks perfectly reasonable to me.  The point I think is that the caller
doesn't have to be very careful about calculating its initial request
size.

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] Allow GiST opcalsses without compress\decompres functions

2017-05-28 Thread Tom Lane
Andrew Borodin  writes:
> Maybe we should make compress\decompress functions optional?

1. You'll need to adjust the documentation (gist.sgml) not just the code.

2. If compress/decompress are omitted, then we could support index-only
scans all the time, that is a no-op fetch function would work.  The
patch should cover that interaction too.

3. Personally I wouldn't bother with the separate compressed[] flags,
just look at OidIsValid(giststate->compressFn[i].fn_oid).

4. I don't think you thought very carefully about the combinations
where only one of the two functions is supplied.  I can definitely
see that compress + no decompress could be sensible.  Less sure
about the other case, but why not allow it?  We could just say that
an omitted function is taken to represent a no-op transformation.

Please add this to the next commitfest.

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 replication - still unstable after all these months

2017-05-28 Thread Erik Rijkers

On 2017-05-26 15:59, Petr Jelinek wrote:

Hmm, I was under the impression that the changes we proposed in the
snapbuild thread fixed your issues, does this mean they didn't? Or the
modified versions of those that were eventually committed didn't? Or 
did

issues reappear at some point?


Here is a bit of info:

Just now (using Mark Kirkwood's version of my test) I had a session 
logging this:


  unknown relation state "w"

which I had never seen before.

This is column srsubstate in pg_subscription_rel.

That session completed successfully ('replica ok'), so it's not 
necessarily a problem.



grepping through my earlier logs (of weeks of intermittent test-runs), I 
found only one more (timestamp 20170525_0125).  Here it occurred in a 
failed session.


No idea what it means.  At the very least this value 'w' is missing from 
the documentation, which only mentions:

  i = initalize
  d = data copy
  s = synchronized
  r = (normal replication)


Erik Rijkers








--
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] Broken hint bits (freeze)

2017-05-28 Thread Amit Kapila
On Sat, May 27, 2017 at 10:18 PM, Vladimir Borodin  wrote:
>
> 26 мая 2017 г., в 21:39, Amit Kapila  написал(а):
>
>  I think you somehow need
> to ensure before switchover that all the WAL is replicated to ensure
> this is not a setup problem.
>
>
> Well, actually clean shutdown of master with exit code 0 from `pg_ctl stop
> -m fast` guarantees that all WAL has been replicated to standby.
>

I don't see any such guarantee in code or docs.  Can you explain what
makes you think that for 'fast' mode exit code 0 is a guarantee that
all the WAL be replicated?

> But just in
> case we also check that "Latest checkpoint's REDO location" from control
> file on old master after shutdown is less than
> pg_last_xlog_replay_location() on standby to be promoted.
>
> And if something would go wrong in above logic, postgres will not let you
> attach old master as a standby of new master.
>

I think it will be possible to attach old master as a standby of new
master as some new operations on the new master can increase its LSN
position to a value greater than what old master has.  Your statement
will make sense if you ensure that you don't allow any new operation
on the new master till old master has attached to it as standby.

> So it is highly probable not a
> setup problem.
>

Yeah, it is quite possible that your setup is perfectly fine and there
is actually some code bug due to which you are facing the problem,
however, it is better to rule out all the possibilities related to the
wrong setup.



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


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


Re: [HACKERS] Extra Vietnamese unaccent rules

2017-05-28 Thread Dang Minh Huong
Hi,

unaccent.patch
Description: Binary data
I am interested in this thread.On May 27, 29 Heisei, at 10:41, Michael Paquier  wrote:On Fri, May 26, 2017 at 5:48 PM, Thomas Munro wrote:Unicode has two ways to represent characters with accents: either withcomposed codepoints like "é" or decomposed codepoints where you say"e" and then "´".  The field "00E2 0301" is the decomposed form ofthat character above.  Our job here is to identify the basic letterthat each composed character contains, by analysing the decomposedfield that you see in that line.  I failed to realise that characterswith TWO accents are described as a composed character with ONE accentplus another accent.Doesn't that depend on the NF operation you are working on? With acanonical decomposition it seems to me that a character with twoaccents can as well be decomposed with one character and two composingcharacter accents (NFKC does a canonical decomposition in one of itssteps).You don't have to worry about decoding that line, it's all done inthat Python script.  The problem is just in the functionis_letter_with_marks().  Instead of just checking if combining_ids[0]is a plain letter, it looks like it should also check ifcombining_ids[0] itself is a letter with marks.  Also get_plain_letterwould need to be able to recurse to extract the "a".Thanks for reporting and lecture about unicode.I attached a patch as the instruction from Thomas. Could you confirm it.Actually, with the recent work that has been done withunicode_norm_table.h which has been to transpose UnicodeData.txt intouser-friendly tables, shouldn't the python script of unaccent/ bereplaced by something that works on this table? This does a canonicaldecomposition but just keeps the first characters with a classordering of 0. So we have basic APIs able to look at UnicodeData.txtand let caller do decision making with the result returned.-- MichaelThanks, i will learning about it.---Dang Minh Huong

Re: [HACKERS] logical replication - still unstable after all these months

2017-05-28 Thread Mark Kirkwood

On 27/05/17 20:30, Erik Rijkers wrote:




Here is what I have:

instances.sh:
  starts up 2 assert enabled sessions

instances_fast.sh:
  alternative to instances.sh
  starts up 2 assert disabled 'fast' sessions

testset.sh
  loop to call pgbench_derail2.sh with varying params

pgbench_derail2.sh
  main test program
  can be called 'standalone'
./pgbench_derail2.sh $scale $clients $duration $date_str
  so for instance this should work:
./pgbench_derail2.sh 25 64 60 20170527_1019
  to remove publication and subscription from sessions, add a 5th 
parameter 'clean'

./pgbench_derail2.sh 1 1 1 1 'clean'

pubsub.sh
  displays replication state. also called by pgbench_derail2.sh
  must be in path

result.sh
  display results
  I keep this in a screen-session as:
  watch -n 20 './result.sh 201705'


Peculiar to my setup also:
  server version at compile time stamped with date + commit hash
  I misuse information_schema.sql_packages  at compile time to store 
patch information

  instances are in $pg_stuff_dir/pg_installations/pgsql.

So you'll have to outcomment a line here and there, and adapt paths, 
ports, and things like that.


It's a bit messy, I should have used perl from the beginning...



Considering it is all shell - pretty nice! I spent a bit of time today 
getting this working in a vanilla Ubuntu 16.04 cloud server. I found a 
few things that didn't work (suspect Erik has some default env variables 
set for ports and databases). These were sufficient to stop logical 
replication working for me at all - due to no dbname specified in the 
subscription connection.


Given I had to make some changes anyway, I moved all the config into one 
place (new file config.sh) - made all the programs use /bin/bash as 
interpreter (/bin/sh just does not work for scripts on Ubuntu), added 
ports and databases as reqd and fixed the need to mess too much with 
PATH (see attached diff).


So running in cloud land now...so for no errors - will update.

regards

Mark
diff -Naur test.orig/config.sh test/config.sh
--- test.orig/config.sh	1970-01-01 12:00:00.0 +1200
+++ test/config.sh	2017-05-28 15:21:33.261701918 +1200
@@ -0,0 +1,13 @@
+#!/bin/bash
+port1=6972
+project1=logical_replication
+port2=6973
+project2=logical_replication2
+pg_stuff_dir=$HOME/pg_stuff
+PATH1=$pg_stuff_dir/pg_installations/pgsql.$project1/bin:$PATH
+PATH2=$pg_stuff_dir/pg_installations/pgsql.$project2/bin:$PATH
+server_dir1=$pg_stuff_dir/pg_installations/pgsql.$project1
+server_dir2=$pg_stuff_dir/pg_installations/pgsql.$project2
+data_dir1=$server_dir1/data
+data_dir2=$server_dir2/data
+db=bench
diff -Naur test.orig/instances_fast.sh test/instances_fast.sh
--- test.orig/instances_fast.sh	2017-05-28 15:18:33.315780487 +1200
+++ test/instances_fast.sh	2017-05-28 15:19:02.511439749 +1200
@@ -1,17 +1,10 @@
-#!/bin/sh
+#!/bin/bash
 
 #assertions on  in $pg_stuff_dir/pg_installations/pgsql./bin
 #assertions off in $pg_stuff_dir/pg_installations/pgsql./bin.fast
 
-port1=6972 project1=logical_replication
-port2=6973 project2=logical_replication2
-pg_stuff_dir=$HOME/pg_stuff
-PATH1=$pg_stuff_dir/pg_installations/pgsql.$project1/bin.fast:$PATH
-PATH2=$pg_stuff_dir/pg_installations/pgsql.$project2/bin.fast:$PATH
-server_dir1=$pg_stuff_dir/pg_installations/pgsql.$project1
-server_dir2=$pg_stuff_dir/pg_installations/pgsql.$project2
-data_dir1=$server_dir1/data
-data_dir2=$server_dir2/data
+. config.sh
+
 options1="
 -c wal_level=logical
 -c max_replication_slots=10
@@ -36,6 +29,6 @@
 -c log_filename=logfile.${project2} 
 -c log_replication_commands=on "
 
-export PATH=$PATH1; PG=$(which postgres); $PG -D $data_dir1 -p $port1 ${options1} &
-export PATH=$PATH2; PG=$(which postgres); $PG -D $data_dir2 -p $port2 ${options2} &
+export PATH=$PATH1; export PG=$(which postgres); $PG -D $data_dir1 -p $port1 ${options1} &
+export PATH=$PATH2; export PG=$(which postgres); $PG -D $data_dir2 -p $port2 ${options2} &
 
diff -Naur test.orig/instances.sh test/instances.sh
--- test.orig/instances.sh	2017-05-28 15:18:33.291780768 +1200
+++ test/instances.sh	2017-05-28 15:19:02.511439749 +1200
@@ -1,17 +1,9 @@
-#!/bin/sh
+#!/bin/bash
 
 #assertions on  in $pg_stuff_dir/pg_installations/pgsql./bin
 #assertions off in $pg_stuff_dir/pg_installations/pgsql./bin.fast
 
-port1=6972 project1=logical_replication
-port2=6973 project2=logical_replication2
-pg_stuff_dir=$HOME/pg_stuff
-PATH1=$pg_stuff_dir/pg_installations/pgsql.$project1/bin:$PATH
-PATH2=$pg_stuff_dir/pg_installations/pgsql.$project2/bin:$PATH
-server_dir1=$pg_stuff_dir/pg_installations/pgsql.$project1
-server_dir2=$pg_stuff_dir/pg_installations/pgsql.$project2
-data_dir1=$server_dir1/data
-data_dir2=$server_dir2/data
+. config.sh
 
 options1="
 -c wal_level=logical
diff -Naur test.orig/pgbench_derail2.sh test/pgbench_derail2.sh
--- test.orig/pgbench_derail2.sh	2017-05-28 15:18:33.363779926 +1200
+++ test/pgbench_derail2.sh	2017-05-28 15:19:02.511439749 +1200
@@ -11,11 +11,13 @@
 #I