Re: [HACKERS] On partitioning

2014-09-18 Thread Amit Langote


> -Original Message-
> From: pgsql-hackers-ow...@postgresql.org [mailto:pgsql-hackers-
> ow...@postgresql.org] On Behalf Of Amit Langote
> Sent: Friday, September 19, 2014 2:13 PM
> To: robertmh...@gmail.com
> Cc: pgsql-hackers@postgresql.org; br...@momjian.us; t...@sss.pgh.pa.us;
> alvhe...@2ndquadrant.com
> Subject: Re: [HACKERS] On partitioning
> 
> Hi,
> 

Apologize for having broken the original thread. :(

This was supposed to in reply to -
http://www.postgresql.org/message-id/CA+Tgmob5DEtO4SbD15q0OQJjyc05cTk8043Utwu_
=xdtvyg...@mail.gmail.com

--
Amit




-- 
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] On partitioning

2014-09-18 Thread Amit Langote
Hi,

I tend to agree with Robert that partitioning should continue using
inheritance based implementation. In addition to his point about reinventing
things it could be pointed out that there are discussions/proposals elsewhere
about building foreign table inheritance capability; having partitioning use
the same general infrastructure would pave a way for including sharding
features more easily in future (perhaps sooner). 

Maybe I am missing something; but isn't it a case that making partitions a
physical implementation detail would make it difficult to support individual
partitions be on different servers (sharding basically)? Moreover, recent FDW
development seems to be headed in direction of substantial core support for
foreign objects/tables; it seems worthwhile for partitioning design to assume
a course so that future sharding feature developers can leverage both. Perhaps
I am just speculating here but I thought of adding this one point to the
discussion.

Having said that, it can also be seen that the subset of inheritance
infrastructure that constitutes partitioning support machinery would have to
be changed considerably if we are now onto partitioning 2.0 here.

--
Amit




-- 
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] Anonymous code block with parameters

2014-09-18 Thread Pavel Stehule
2014-09-18 22:35 GMT+02:00 Josh Berkus :

> On 09/18/2014 01:29 PM, Vik Fearing wrote:
> > On 09/18/2014 10:16 PM, Hannu Krosing wrote:
> >>> WITH
> >>>  FUNCTION f1(a int) RETURNS int AS $$ .. $$ LANGUAGE plpgsql,
> >>>  FUNCTION f2(a int) RETURNS SETOF int AS $$ .. $$ LANGUAGE
> >>> plpgsql,
> >>>   SELECT f1(x) FROM f2(z) LATERAL 
> >>>
> >>> We can generalize WITH clause, so there SEQENCES, VIEWS, .. can be
> >>> defined for "single usage"
> >> +2
> >>
> >> I just proposed the same thing in another branch of this discussion
> >> before reading this :)
> >>
> >> I guess it proves (a little) that WITH is the right place to do these
> >> kind of things ...
> >
> > I've been wanting this syntax for a few years now, so I certainly vote
> > for it.
> >
>
> Just to clarify: I want the WITH syntax for different purposes.
> However, I *also* want DO $$ ... $$ USING ( ).  Those are two separate,
> different features with different use-cases.
>

+1 as parametrized (read only) DO statement


>
> --
> Josh Berkus
> PostgreSQL Experts Inc.
> http://pgexperts.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] Final Patch for GROUPING SETS

2014-09-18 Thread Andrew Gierth
> "Marti" == Marti Raudsepp  writes:

 Marti> Since you were asking for feedback on the EXPLAIN output on
 Marti> IRC, I'd weigh in and say that having the groups on separate
 Marti> lines would be significantly more readable.

I revisited the explain output a bit and have come up with these
(surrounding material trimmed for clarity):

(text format)

 GroupAggregate  (cost=1122.39..1197.48 rows=9 width=8)
   Group Key: two, four
   Group Key: two
   Group Key: ()
   ->  ...

(xml format)


  Aggregate
  Sorted
  1122.39
  1197.48
  9
  8
  

  two
  four


  two



  
  ...

(json format)

"Plan": {
  "Node Type": "Aggregate",
  "Strategy": "Sorted",
  "Startup Cost": 1122.39,
  "Total Cost": 1197.48,
  "Plan Rows": 9,
  "Plan Width": 8,
  "Grouping Sets": [
["two", "four"],
["two"],
[]
  ],
  "Plans": [...]

(yaml format)

- Plan: 
Node Type: "Aggregate"
Strategy: "Sorted"
Startup Cost: 1122.39
Total Cost: 1197.48
Plan Rows: 9
Plan Width: 8
Grouping Sets: 
  - - "two"
- "four"
  - - "two"
  - 
Plans: ...

Opinions? Any improvements?

I'm not entirely happy with what I had to do with the json and
(especially) the YAML output code in order to make this work.  There
seemed no obvious way to generate nested unlabelled structures in
either using the existing Explain* functions, and for the YAML case
the best output structure to produce was entirely non-obvious (and
trying to read the YAML spec made my head explode).

 Marti> Do you think it would be reasonable to normalize single-set
 Marti> grouping sets into a normal GROUP BY?

It's certainly possible, though it would seem somewhat odd to write
queries that way. Either the parser or the planner could do that;
would you want the original syntax preserved in views, or wouldn't
that matter?

 Marti> I'd expect GROUP BY () to be fully equivalent to having no
 Marti> GROUP BY clause, but there's a difference in explain
 Marti> output. The former displays "Grouping Sets: ()" which is odd,
 Marti> since none of the grouping set keywords were used.

That's an implementation artifact, in the sense that we preserve the
fact that GROUP BY () was used by using an empty grouping set. Is it
a problem, really, that it shows up that way in explain?

-- 
Andrew (irc:RhodiumToad)


-- 
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] Anonymous code block with parameters

2014-09-18 Thread Hannu Krosing
On 09/19/2014 12:14 AM, Hannu Krosing wrote:
> On 09/18/2014 10:40 PM, Marko Tiikkaja wrote:
>> On 2014-09-18 10:29 PM, Vik Fearing wrote:
>>> On 09/18/2014 10:16 PM, Hannu Krosing wrote:
 I guess it proves (a little) that WITH is the right place to do these
 kind of things ...
>>> I've been wanting this syntax for a few years now, so I certainly vote
>>> for it.
>> I've also been wanting do to something like:
>>
>>   WITH mytyp AS (a int, b int, c int)
>>   SELECT (tup).* FROM
>>   (
>> SELECT CASE WHEN .. THEN ROW(1,2,3)::mytyp
>> WHEN .. THEN ROW(2,3,4)
>> ELSE ROW (3,4,5) END AS tup
>> FROM ..
>>   ) ss
> +1
Though it would be even nicer to have fully in-line type definition

SELECT (tup).* FROM
  (
SELECT CASE WHEN .. THEN ROW(1,2,3)::(a int, b text, c int2)
WHEN .. THEN ROW(2,3,4)
ELSE ROW (3,4,5) END AS tup
FROM ..
  ) ss

or an incomplete type with names, as types can be given in ROW

SELECT (tup).* FROM
  (
SELECT CASE WHEN .. THEN ROW(1,2::text,3::int2)::(a, b, c)
WHEN .. THEN ROW(2,3,4)
ELSE ROW (3,4,5) END AS tup
FROM ..
  ) ss

or just normal select query syntax:

SELECT (tup).* FROM
  (
SELECT CASE WHEN .. THEN ROW(1 AS a,2::text AS b,3::int2 AS c)
WHEN .. THEN ROW(2,3,4)
ELSE ROW (3,4,5) END AS tup
FROM ..
  ) ss




Cheers

-- 
Hannu Krosing
PostgreSQL Consultant
Performance, Scalability and High Availability
2ndQuadrant Nordic OÜ



-- 
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] Anonymous code block with parameters

2014-09-18 Thread Hannu Krosing
On 09/18/2014 10:40 PM, Marko Tiikkaja wrote:
> On 2014-09-18 10:29 PM, Vik Fearing wrote:
>> On 09/18/2014 10:16 PM, Hannu Krosing wrote:
>>> I guess it proves (a little) that WITH is the right place to do these
>>> kind of things ...
>>
>> I've been wanting this syntax for a few years now, so I certainly vote
>> for it.
>
> I've also been wanting do to something like:
>
>   WITH mytyp AS (a int, b int, c int)
>   SELECT (tup).* FROM
>   (
> SELECT CASE WHEN .. THEN ROW(1,2,3)::mytyp
> WHEN .. THEN ROW(2,3,4)
> ELSE ROW (3,4,5) END AS tup
> FROM ..
>   ) ss
+1
>
>
> .marko
>
>



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


Re: [HACKERS] [GENERAL] [SQL] pg_multixact issues

2014-09-18 Thread Adrian Klaver

On 09/18/2014 10:22 AM, Dev Kumkar wrote:

On Thu, Sep 18, 2014 at 6:20 PM, Dev Kumkar mailto:devdas.kum...@gmail.com>> wrote:

On Thu, Sep 18, 2014 at 4:03 PM, Andres Freund
mailto:and...@2ndquadrant.com>> wrote:

I don't think that's relevant for you.

Did you upgrade the database using pg_upgrade?


That's correct! No, there is no upgrade here.


The above sentence is not clear to me.

Did you run pg_upgrade to get the data into the database?

If not, how did the database get populated?




Can you show pg_controldata output and the output of 'SELECT oid,
datname, relfrozenxid, age(relfrozenxid), relminmxid FROM
pg_database;'?


Here are the details:
  oid   datname datfrozenxidage(datfrozenxid)datminmxid
16384 myDB1673 10872259 1

Additionally wanted to mention couple more points here:
When I try to run "vacuum full" on this machine then facing
following issue:
  INFO:  vacuuming "myDB.mytable"
  ERROR:  MultiXactId 3622035 has not been created yet --
apparent wraparound

No Select statements are working on this table, is the table corrupt?


Any inputs/hints/tips here?


Have you run the query from here?:

http://www.postgresql.org/docs/9.3/interactive/release-9-3-5.html

WITH list(file) AS (SELECT * FROM pg_ls_dir('pg_multixact/offsets'))
SELECT EXISTS (SELECT * FROM list WHERE file = '') AND
   NOT EXISTS (SELECT * FROM list WHERE file = '0001') AND
   NOT EXISTS (SELECT * FROM list WHERE file = '') AND
   EXISTS (SELECT * FROM list WHERE file != '')
   AS file__removal_required;




--
Adrian Klaver
adrian.kla...@aklaver.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] Anonymous code block with parameters

2014-09-18 Thread Marko Tiikkaja

On 2014-09-18 10:29 PM, Vik Fearing wrote:

On 09/18/2014 10:16 PM, Hannu Krosing wrote:

I guess it proves (a little) that WITH is the right place to do these
kind of things ...


I've been wanting this syntax for a few years now, so I certainly vote
for it.


I've also been wanting do to something like:

  WITH mytyp AS (a int, b int, c int)
  SELECT (tup).* FROM
  (
SELECT CASE WHEN .. THEN ROW(1,2,3)::mytyp
WHEN .. THEN ROW(2,3,4)
ELSE ROW (3,4,5) END AS tup
FROM ..
  ) ss


.marko


--
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] Anonymous code block with parameters

2014-09-18 Thread Josh Berkus
On 09/18/2014 01:29 PM, Vik Fearing wrote:
> On 09/18/2014 10:16 PM, Hannu Krosing wrote:
>>> WITH
>>>  FUNCTION f1(a int) RETURNS int AS $$ .. $$ LANGUAGE plpgsql,
>>>  FUNCTION f2(a int) RETURNS SETOF int AS $$ .. $$ LANGUAGE
>>> plpgsql,
>>>   SELECT f1(x) FROM f2(z) LATERAL 
>>>
>>> We can generalize WITH clause, so there SEQENCES, VIEWS, .. can be
>>> defined for "single usage"
>> +2
>>
>> I just proposed the same thing in another branch of this discussion
>> before reading this :)
>>
>> I guess it proves (a little) that WITH is the right place to do these
>> kind of things ...
> 
> I've been wanting this syntax for a few years now, so I certainly vote
> for it.
> 

Just to clarify: I want the WITH syntax for different purposes.
However, I *also* want DO $$ ... $$ USING ( ).  Those are two separate,
different features with different use-cases.

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.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] Anonymous code block with parameters

2014-09-18 Thread Vik Fearing
On 09/18/2014 10:16 PM, Hannu Krosing wrote:
> On 09/18/2014 02:37 PM, Pavel Stehule wrote:
>>
>> if we would to need a "single use" function, then we should to
>> implement it, and we should not to rape some different objects. Some,
>> what has behave like function should be function.
>>
>> After some thinking, probably CTE design can be only one frame, where
>> we can do it
>>
>> WITH
>>  FUNCTION f1(a int) RETURNS int AS $$ .. $$ LANGUAGE plpgsql,
>>  FUNCTION f2(a int) RETURNS SETOF int AS $$ .. $$ LANGUAGE
>> plpgsql,
>>   SELECT f1(x) FROM f2(z) LATERAL 
>>
>> We can generalize WITH clause, so there SEQENCES, VIEWS, .. can be
>> defined for "single usage"
> +2
> 
> I just proposed the same thing in another branch of this discussion
> before reading this :)
> 
> I guess it proves (a little) that WITH is the right place to do these
> kind of things ...

I've been wanting this syntax for a few years now, so I certainly vote
for it.
-- 
Vik


-- 
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] Anonymous code block with parameters

2014-09-18 Thread Josh Berkus
On 09/18/2014 01:10 PM, Hannu Krosing wrote:
> One possible syntax would be extending WITH to somehow enable on-spot
> functions in addition to on-spot views
> 
> WITH FUNCTION myfunc(...) RETURNS TABLE(...) LANGUAGE plpgsql AS $$
> ...
> $$
> SELECT f.*
>   FROM myfunc(x,y,z);

Oh!  Awesome!

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.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] Anonymous code block with parameters

2014-09-18 Thread Hannu Krosing
On 09/18/2014 02:37 PM, Pavel Stehule wrote:
>
> if we would to need a "single use" function, then we should to
> implement it, and we should not to rape some different objects. Some,
> what has behave like function should be function.
>
> After some thinking, probably CTE design can be only one frame, where
> we can do it
>
> WITH
>  FUNCTION f1(a int) RETURNS int AS $$ .. $$ LANGUAGE plpgsql,
>  FUNCTION f2(a int) RETURNS SETOF int AS $$ .. $$ LANGUAGE
> plpgsql,
>   SELECT f1(x) FROM f2(z) LATERAL 
>
> We can generalize WITH clause, so there SEQENCES, VIEWS, .. can be
> defined for "single usage"
+2

I just proposed the same thing in another branch of this discussion
before reading this :)

I guess it proves (a little) that WITH is the right place to do these
kind of things ...


Cheers

-- 
Hannu Krosing
PostgreSQL Consultant
Performance, Scalability and High Availability
2ndQuadrant Nordic OÜ



-- 
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] Anonymous code block with parameters

2014-09-18 Thread Hannu Krosing
On 09/18/2014 08:41 PM, Andrew Dunstan wrote:
>
> On 09/18/2014 07:40 AM, Andres Freund wrote:
>> On 2014-09-17 22:17:22 +0200, Pavel Stehule wrote:
>>> 2014-09-17 22:07 GMT+02:00 Vik Fearing :
>>>
 On 09/16/2014 10:09 AM, Heikki Linnakangas wrote:
> On 09/16/2014 10:57 AM, Craig Ringer wrote:
>> On 09/16/2014 03:15 PM, Pavel Stehule wrote:
>>
>>> Why we don't introduce a temporary functions instead?
>> I think that'd be a lot cleaner and simpler. It's something I've
>> frequently wanted, and as Hekki points out it's already possible by
>> creating the function in pg_temp, there just isn't the syntax
>> sugar for
>> "CREATE TEMPORARY FUNCTION".
>>
>> So why not just add "CREATE TEMPORARY FUNCTION"?
> Sure, why not.
 Because you still have to do

  SELECT pg_temp.my_temp_function(blah);

 to execute it.

>>> this problem should be solvable. I can to use a temporary tables
>>> without
>>> using pg_temp schema.
>> I fail to see why that is so much preferrable for you to passing
>> parameter to DO?
>>
>> 1) You need to think about unique names for functions
>> 2) Doesn't work on HOT STANDBYs
>> 3) Causes noticeable amount of catalog bloat
>> 4) Is about a magnitude or two more expensive
>>
>> So yes, TEMPORARY FUNCTION would be helpful. But it's simply a different
>> feature.
>>
>
>
> +1
>
> If my memory isn't failing, when we implemented DO there were
> arguments for this additional feature, but we decided that it wouldn't
> be done at least on the first round. But we've had DO for a while and
> it's proved its worth. So I think now is a perfect time to revisit the
> issue.
One possible syntax would be extending WITH to somehow enable on-spot
functions in addition to on-spot views

WITH FUNCTION myfunc(...) RETURNS TABLE(...) LANGUAGE plpgsql AS $$
...
$$
SELECT f.*
  FROM myfunc(x,y,z);


Cheers

-- 
Hannu Krosing
PostgreSQL Consultant
Performance, Scalability and High Availability
2ndQuadrant Nordic OÜ



-- 
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] Collations and Replication; Next Steps

2014-09-18 Thread Peter Geoghegan
On Thu, Sep 18, 2014 at 6:51 AM, Heikki Linnakangas
 wrote:
> The same it works with libxml, openssl, libreadline and all the other
> libraries you can build with.

I like the comparison with libxml. If we were to adopt ICU, it would
be as a core component that makes collation versioning work, that in
practice all packages use. It wouldn't actually be mandatory, but
almost universally available in practice.

I really think that long term, relying on the OS collations is not a
good plan. It's a big contributing factor to our reticence on the
question of when two different systems should be considered compatible
for the purposes of physical replication. Not that I'm volunteering to
work on it!
-- 
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] Anonymous code block with parameters

2014-09-18 Thread Andrew Dunstan


On 09/18/2014 07:40 AM, Andres Freund wrote:

On 2014-09-17 22:17:22 +0200, Pavel Stehule wrote:

2014-09-17 22:07 GMT+02:00 Vik Fearing :


On 09/16/2014 10:09 AM, Heikki Linnakangas wrote:

On 09/16/2014 10:57 AM, Craig Ringer wrote:

On 09/16/2014 03:15 PM, Pavel Stehule wrote:


Why we don't introduce a temporary functions instead?

I think that'd be a lot cleaner and simpler. It's something I've
frequently wanted, and as Hekki points out it's already possible by
creating the function in pg_temp, there just isn't the syntax sugar for
"CREATE TEMPORARY FUNCTION".

So why not just add "CREATE TEMPORARY FUNCTION"?

Sure, why not.

Because you still have to do

 SELECT pg_temp.my_temp_function(blah);

to execute it.


this problem should be solvable. I can to use a temporary tables without
using pg_temp schema.

I fail to see why that is so much preferrable for you to passing
parameter to DO?

1) You need to think about unique names for functions
2) Doesn't work on HOT STANDBYs
3) Causes noticeable amount of catalog bloat
4) Is about a magnitude or two more expensive

So yes, TEMPORARY FUNCTION would be helpful. But it's simply a different
feature.




+1

If my memory isn't failing, when we implemented DO there were arguments 
for this additional feature, but we decided that it wouldn't be done at 
least on the first round. But we've had DO for a while and it's proved 
its worth. So I think now is a perfect time to revisit the issue.


cheers

andrew


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


Re: [HACKERS] jsonb format is pessimal for toast compression

2014-09-18 Thread Heikki Linnakangas

On 09/18/2014 07:53 PM, Josh Berkus wrote:

On 09/16/2014 08:45 PM, Tom Lane wrote:

We're somewhat comparing apples and oranges here, in that I pushed my
approach to something that I think is of committable quality (and which,
not incidentally, fixes some existing bugs that we'd need to fix in any
case); while Heikki's patch was just proof-of-concept.  It would be worth
pushing Heikki's patch to committable quality so that we had a more
complete understanding of just what the complexity difference really is.


Is anyone actually working on this?

If not, I'm voting for the all-lengths patch so that we can get 9.4 out
the door.


I'll try to write a more polished patch tomorrow. We'll then see what it 
looks like, and can decide if we want it.


- Heikki



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


Re: [HACKERS] [GENERAL] [SQL] pg_multixact issues

2014-09-18 Thread Dev Kumkar
On Thu, Sep 18, 2014 at 6:20 PM, Dev Kumkar  wrote:

> On Thu, Sep 18, 2014 at 4:03 PM, Andres Freund 
> wrote:
>
>> I don't think that's relevant for you.
>>
>> Did you upgrade the database using pg_upgrade?
>>
>
> That's correct! No, there is no upgrade here.
>
>
>> Can you show pg_controldata output and the output of 'SELECT oid,
>> datname, relfrozenxid, age(relfrozenxid), relminmxid FROM pg_database;'?
>>
>
> Here are the details:
>  oid   datname datfrozenxidage(datfrozenxid)datminmxid
> 16384 myDB1673 10872259 1
>
> Additionally wanted to mention couple more points here:
> When I try to run "vacuum full" on this machine then facing following
> issue:
>  INFO:  vacuuming "myDB.mytable"
>  ERROR:  MultiXactId 3622035 has not been created yet -- apparent
> wraparound
>
> No Select statements are working on this table, is the table corrupt?
>

Any inputs/hints/tips here?


Re: [HACKERS] jsonb format is pessimal for toast compression

2014-09-18 Thread Josh Berkus
On 09/16/2014 08:45 PM, Tom Lane wrote:
> We're somewhat comparing apples and oranges here, in that I pushed my
> approach to something that I think is of committable quality (and which,
> not incidentally, fixes some existing bugs that we'd need to fix in any
> case); while Heikki's patch was just proof-of-concept.  It would be worth
> pushing Heikki's patch to committable quality so that we had a more
> complete understanding of just what the complexity difference really is.

Is anyone actually working on this?

If not, I'm voting for the all-lengths patch so that we can get 9.4 out
the door.

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com


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


[HACKERS] BUG: *FF WALs under 9.2 (WAS: .ready files appearing on slaves)

2014-09-18 Thread Jehan-Guillaume de Rorthais
Hi hackers,

We spend some time with Guillaume Lelarge studying this issue.

CreateRestartPoint() calls RemoveOldXlogFiles() to drop/recycle old WALs. This
one is calling XLogArchiveCheckDone() to check if the given WAL can be dropped.
As our slave has "archive_mode" & "archive_command" set, XLogArchivingActive()
returns true, forcing XLogArchiveCheckDone() to look for a ".done" file. If the
corresponding ".done" file doesn't exist, XLogArchiveCheckDone() calls
XLogArchiveNotify() to actually create the ".ready" file.


Now, on a standby, we are supposed to create the ".done" files by calling
XLogArchiveForceDone() either after a successful restore_command or when the
streaming replication finishes and flushes a WAL. It seems like the root cause
of this trouble: maybe a race condition where XLogArchiveForceDone() would not
be called for a WAL. As the ".done" file is never created, we end up calling
XLogArchiveNotify() to create a ".ready" and this WAL will never be archived by
the slave, sitting in the pg_xlog dir.

But worst, we experience this under two 9.2.8 clusters and realized
this version is not supposed to have 256 segments per WAL, finishing a WAL to
the *FE one. I'm quoting back the history of my previous mail as we clearly
have some *FF files on the slave side!

> >   0001040E00FF
> >   0001041400DA
> >   0001046E00FF
> >   0001047000FF
> >   00010485000F
> >   000104850010
> >   [...normal WAL sequence...]
> >   000104850052

Note that it seems to happen only during streaming replication, not in pure Log
Shipping replication. We added the debug message on the master side to make
sure it never archive *FF file

We kept the WAL files and log files for further analysis. How can we help
regarding this issue?

Regards,


On Mon, 15 Sep 2014 17:37:24 +0200
Jehan-Guillaume de Rorthais  wrote:

> Hi hackers,
> 
> An issue that seems related to this has been posted on pgsql-admin. See:
> 
>   
> http://www.postgresql.org/message-id/caas3tylnxaydz0+zhxlpdvtovhqovr+jsphp30o8kvwqqs0...@mail.gmail.com
> 
> How can we help on this issue?
> 
> Cheers,
> 
> On Thu, 4 Sep 2014 17:50:36 +0200
> Jehan-Guillaume de Rorthais  wrote:
> 
> > Hi hackers,
> > 
> > Since few months, we occasionally see .ready files appearing on some slave
> >  instances from various context. The two I have in mind are under 9.2.x.
> > 
> > I tried to investigate a bit. These .ready files are created when a WAL file
> > from pg_xlog has no corresponding file in pg_xlog/archive_status. I could
> > easily experience this by deleting such a file: it is created again at the
> > next restartpoint or checkpoint received from the master.
> > 
> > Looking at the WAL in pg_xlog folder corresponding to these .ready files,
> > they are all much older than the current WAL "cycle" in both mtime and name
> > logic sequence. As instance on one of these box we have currently 6 of
> > those "ghost" WALs:
> > 
> >   00021E5300FF
> >   00021F1800FF
> >   0002204700FF
> >   000220BF00FF
> >   0002214000FF
> >   0002237000FF
> >   0002255D00A8
> >   0002255D00A9
> >   [...normal WAL sequence...]
> >   0002255E009D
> > 
> > And on another box:
> > 
> >   0001040E00FF
> >   0001041400DA
> >   0001046E00FF
> >   0001047000FF
> >   00010485000F
> >   000104850010
> >   [...normal WAL sequence...]
> >   000104850052
> > 
> > So it seems for some reasons, these old WALs were "forgotten" by the
> > restartpoint mechanism when they should have been recylced/deleted. 
> > 
> > For one of these servers, I could correlate this with some brutal
> > disconnection of the streaming replication appearing in its logs. But there
> > was no known SR disconnection on the second one.
> > 
> > Any idea about this weird behaviour? What can we do to help you investigate
> > further?
> > 
> > Regards,
> 
> 
> 



-- 
Jehan-Guillaume de Rorthais
Dalibo
http://www.dalibo.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] Options OUTPUT_PLUGIN_* controlling format are confusing (Was: Misleading error message in logical decoding)

2014-09-18 Thread Michael Paquier
On Thu, Sep 18, 2014 at 9:56 AM, Andres Freund  wrote:
> On 2014-09-18 09:50:38 -0500, Michael Paquier wrote:
>> > Do you see the difference between what your doc patch states and the
>> > explanation I've given nearby in this thread?
>> Perhaps that's the lack of documentation...
>
> Man. I've explained it to you about three times. The previous attempts
> at doing so didn't seem to help. If my explanations don't explain it so
> you can understand it adding them to the docs won't change a thing.
> That's why I ask whether you see the difference?
Urg sorry for the misunderstanding. The patch stated that this
parameter only influences the output of the SQL functions while it
defines if "the output plugin requires the output method to support
binary data"?
-- 
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] Options OUTPUT_PLUGIN_* controlling format are confusing (Was: Misleading error message in logical decoding)

2014-09-18 Thread Andres Freund
On 2014-09-18 09:50:38 -0500, Michael Paquier wrote:
> > Do you see the difference between what your doc patch states and the
> > explanation I've given nearby in this thread?
> Perhaps that's the lack of documentation...

Man. I've explained it to you about three times. The previous attempts
at doing so didn't seem to help. If my explanations don't explain it so
you can understand it adding them to the docs won't change a thing.
That's why I ask whether you see the difference?

Greetings,

Andres Freund

-- 
 Andres Freund 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] Options OUTPUT_PLUGIN_* controlling format are confusing (Was: Misleading error message in logical decoding)

2014-09-18 Thread Michael Paquier
On Thu, Sep 18, 2014 at 9:23 AM, Andres Freund  wrote:
> On 2014-09-18 09:13:48 -0500, Michael Paquier wrote:
>> > On Thu, Sep 18, 2014 at 2:18 AM, Andres Freund  
>> > wrote:
>> >> Improving the docs here is on my roadmap, but I don't see the benefit of
>> >> this.
>> Btw, I sent a couple of weeks back a patch that was an attempt to
>> improve this portion of the docs, feel free to have a look if that
>> helps :)
>> http://www.postgresql.org/message-id/CAB7nPqQDxcHNEBhG71nL6jDGQji9m=fqeooks1_ylbfsts4...@mail.gmail.com
>
> Uh, I know - I commented extensively on that thread. Your doc patch is
> wrong, so I won't apply it ;) and also can't really use it as a basis...
Fine. No problem.

> I'd understood
> cab7npqrcocrrf84vhrvdrk8+9rgzbrfbkwmri9_79tfkmz4...@mail.gmail.com in
> that thread to mean we were on one page. But the message referenced by
> you above (sent later) and this discussion seems to indicate that that's
> not the case.

> Do you see the difference between what your doc patch states and the
> explanation I've given nearby in this thread?
Perhaps that's the lack of documentation... Putting on the plugin
developer hat (if there's one), things are not clear for users. And
the thought that a mandatory option value within the plugin that is
only linked to the SQL interface is somewhat non-intuitive. It would
have been perhaps more interesting to allow users to specify a list of
data types that are allowed as output instead. I guess that's just
food for thoughts though...
-- 
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] Options OUTPUT_PLUGIN_* controlling format are confusing (Was: Misleading error message in logical decoding)

2014-09-18 Thread Andres Freund
On 2014-09-18 09:13:48 -0500, Michael Paquier wrote:
> > On Thu, Sep 18, 2014 at 2:18 AM, Andres Freund  
> > wrote:
> >> Improving the docs here is on my roadmap, but I don't see the benefit of
> >> this.
> Btw, I sent a couple of weeks back a patch that was an attempt to
> improve this portion of the docs, feel free to have a look if that
> helps :)
> http://www.postgresql.org/message-id/CAB7nPqQDxcHNEBhG71nL6jDGQji9m=fqeooks1_ylbfsts4...@mail.gmail.com

Uh, I know - I commented extensively on that thread. Your doc patch is
wrong, so I won't apply it ;) and also can't really use it as a basis...

I'd understood
cab7npqrcocrrf84vhrvdrk8+9rgzbrfbkwmri9_79tfkmz4...@mail.gmail.com in
that thread to mean we were on one page. But the message referenced by
you above (sent later) and this discussion seems to indicate that that's
not the case.

Do you see the difference between what your doc patch states and the
explanation I've given nearby in this thread?

Greetings,

Andres Freund

-- 
 Andres Freund 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] Options OUTPUT_PLUGIN_* controlling format are confusing (Was: Misleading error message in logical decoding)

2014-09-18 Thread Michael Paquier
> On Thu, Sep 18, 2014 at 2:18 AM, Andres Freund  wrote:
>> Improving the docs here is on my roadmap, but I don't see the benefit of
>> this.
Btw, I sent a couple of weeks back a patch that was an attempt to
improve this portion of the docs, feel free to have a look if that
helps :)
http://www.postgresql.org/message-id/CAB7nPqQDxcHNEBhG71nL6jDGQji9m=fqeooks1_ylbfsts4...@mail.gmail.com
-- 
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] Options OUTPUT_PLUGIN_* controlling format are confusing (Was: Misleading error message in logical decoding)

2014-09-18 Thread Andres Freund
On 2014-09-18 08:57:26 -0500, Michael Paquier wrote:
> On Thu, Sep 18, 2014 at 2:18 AM, Andres Freund  wrote:
> > The point is that operating with byteas on SQL level is freaking
> > painful.
> An example perhaps? I fail to see why it is related to the fact that a
> user could simply use that to fetch changes in bytea from a slot:
> select data::bytea from pg_logical_slot_get_changes('foo', NULL, NULL);

Uh. Because that'd mean that pg_logical_slot_get_changes() would return
text, that's not actually text? I.e. it'll contain 0 bytes and things
outside the server's encoding? Not good.

> That's perhaps not more simple than using the binary functions, which
> actually explain their existence because of the textual/binary format
> context with OUTPUT_PLUGIN_*, but that's possible. It is as well
> possible to pay quite a lot with custom data types and casts to
> transform textual changes (not to mention that a plugin could send out
> bytea changes by itself through the COPY string). Looking at the code,
> the only difference between textual and binary is an assertion check
> using the database encoding, something unlikely to be triggered on
> production systems btw.

No. There's a *SIGNIFICANT* additional difference. If you use the *non
binary* get/peek_changes on a output plugin which has declared (and we
normally believe C code) that it produces binary output you will get an
error.

Again:
The output plugin *declares* whether it returns binary data (i.e. data
that's not in the server's encoding and/or possibly contains 0
bytes.). The textual get/peek_changes cannot accept changes from plugins
declaring that. The binary variant can accept changes regardless of
that.

I have the feeling that you have some fundamentally different
understanding of this in your head and aren't really following my
explanations.

Greetings,

Andres Freund

-- 
 Andres Freund 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] Options OUTPUT_PLUGIN_* controlling format are confusing (Was: Misleading error message in logical decoding)

2014-09-18 Thread Michael Paquier
On Thu, Sep 18, 2014 at 2:18 AM, Andres Freund  wrote:
> The point is that operating with byteas on SQL level is freaking
> painful.
An example perhaps? I fail to see why it is related to the fact that a
user could simply use that to fetch changes in bytea from a slot:
select data::bytea from pg_logical_slot_get_changes('foo', NULL, NULL);
That's perhaps not more simple than using the binary functions, which
actually explain their existence because of the textual/binary format
context with OUTPUT_PLUGIN_*, but that's possible. It is as well
possible to pay quite a lot with custom data types and casts to
transform textual changes (not to mention that a plugin could send out
bytea changes by itself through the COPY string). Looking at the code,
the only difference between textual and binary is an assertion check
using the database encoding, something unlikely to be triggered on
production systems btw.

>> I am raising this point before the 9.4 ship sails, thinking long-term
>> and to faciliate the maintenance of existing code. Attached is a patch
>> that simplifies the current logical decoding API regarding that.
>
> Sorry, -1.
>
> Improving the docs here is on my roadmap, but I don't see the benefit of
> this.
Well, there is no actual benefit. That's only a logical reasoning to
keep the interface as simple as possible without impacting the feature
range. Enforcing an option with arbitrary values that only has effect
for non-replication connections is unintuitive for a plugin structure
that is dedicated for both non-replication and replication protocols
(using replication protocol offers more flexibility, so I'd expect it
to be more widely used than the SQL interface btw). If docs are
improved, it should at least be clearly stated what is the benefit of
one value to the other when fetching changes using the get/peek
functions.
-- 
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] Collations and Replication; Next Steps

2014-09-18 Thread Heikki Linnakangas

On 09/18/2014 04:12 PM, Oleg Bartunov wrote:

On Thu, Sep 18, 2014 at 3:25 PM, Martijn van Oosterhout 
wrote:


On Thu, Sep 18, 2014 at 01:35:10PM +0900, Tatsuo Ishii wrote:

In my understanding PostgreSQL's manual MUST include the ICU license
term (this is not a problem).  What I am not so sure is, any software
uses PostgreSQL also MUST include the ICU license or not. If yes, I
think this is surely a problem.


Only if we're thinking of distributing it. If the user gets ICU from
their distribution then there is no need to list the licence (just like
we don't need to mention the licence of glibc).  We only need link
against it, not distribute it.


I understand how it'd works with extension, but not with core.


The same it works with libxml, openssl, libreadline and all the other 
libraries you can build with.


- Heikki



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


Re: [HACKERS] Collations and Replication; Next Steps

2014-09-18 Thread Oleg Bartunov
On Thu, Sep 18, 2014 at 3:25 PM, Martijn van Oosterhout 
wrote:

> On Thu, Sep 18, 2014 at 01:35:10PM +0900, Tatsuo Ishii wrote:
> > In my understanding PostgreSQL's manual MUST include the ICU license
> > term (this is not a problem).  What I am not so sure is, any software
> > uses PostgreSQL also MUST include the ICU license or not. If yes, I
> > think this is surely a problem.
>
> Only if we're thinking of distributing it. If the user gets ICU from
> their distribution then there is no need to list the licence (just like
> we don't need to mention the licence of glibc).  We only need link
> against it, not distribute it.
>

I understand how it'd works with extension, but not with core.

>
> Have a nice day,
> --
> Martijn van Oosterhout  http://svana.org/kleptog/
> > He who writes carelessly confesses thereby at the very outset that he
> does
> > not attach much importance to his own thoughts.
>-- Arthur Schopenhauer
>
> -BEGIN PGP SIGNATURE-
> Version: GnuPG v1.4.10 (GNU/Linux)
>
> iQIVAwUBVBrBQkvt++dL5i1EAQhDehAAiQg7iLKficvXMSfAvwR+8Qp7yTG3wiNu
> oZ5ieomZCYhlTpXvHae9dWTJPKehjiCCy/zjVyLpFvfHQVptadjBVefkIFSS/d+V
> Y7X3gPI3d+8Tc+ZGVZF/CX/5eG9iPgKbsDRpK0zs5j+C4D1HYjxLFf4jWYI/gTXN
> Abfr6taSi4YrSAw/4bSMlSQMFDU/wmLx175R4f8j2CvEYmspgXe89i4QU2V14m6Y
> bB+zGhkxxJZAubTq3UcPzDDeX3FH4KqS/4NTSZ1V1ceIWo3r9jRPLHpceAvQlHwP
> e4eNnRkFZbTeLlOUcvd7N7qkEc2kDYEGXKyaNqr868N022mFiYx4AHwOU/U/dbmm
> Xw22FpSTwkokmwugohr7wrL7tUJV7NtHrcEUyPd/2cuddIRvO2H2iV4wqR6ct0TZ
> 2RCd1b7bIKq+ywrGkySW1xplMhGmGygfPnUzqzlZ2f1YxcmK6PNMnpxldy5nvl3V
> 2rNnPOoWS3H+R6aE31sSGH+Gl9w6J4lvpPiTAwx8pGoBPi4fWWqvwHUPp7FNqsAO
> 8fjo00+MN9Vbg0YsqHiE6oCp2pKs3BJy3IHfZiw2nefh9UcEV29666atUBJjb3bw
> hO65Km7uzoacX+WKm0XdmaQhdUwVJKIFFoH3sxnawA6+CUr4M8mUDtQicnd/ajRo
> HX0lbgRk9z4=
> =31r4
> -END PGP SIGNATURE-
>
>


Re: [HACKERS] Anonymous code block with parameters

2014-09-18 Thread Pavel Stehule
2014-09-18 13:59 GMT+02:00 Pavel Stehule :

>
>
> 2014-09-18 13:53 GMT+02:00 Andres Freund :
>
>> On 2014-09-18 13:51:56 +0200, Pavel Stehule wrote:
>> > 2014-09-18 13:48 GMT+02:00 Andres Freund :
>> >
>> > > On 2014-09-18 13:44:47 +0200, Pavel Stehule wrote:
>> > > Isn't being able to do this on a standby a fundamental enough
>> advantage?
>> > > Being significantly cheaper? Needing fewer roundtrips?
>> > >
>> >
>> > no, I don't need more. My opinion is, so this proposal has no real
>> benefit,
>> > but will do implement redundant functionality.
>>
>> FFS: What's redundant about being able to do this on a standby?
>>
>
> Is it solution for standby? It is necessary? You can have a functions on
> master.
>
> Is not higher missfeature temporary tables on stanby?
>
> again: I am not against to DO paramaterization. I am against to implement
> DO with complexity like functions. If we have a problem with standby, then
> we have to fix it correctly. There is a issue with temp tables, temp
> sequences, temp functions.
>

if we would to need a "single use" function, then we should to implement
it, and we should not to rape some different objects. Some, what has behave
like function should be function.

After some thinking, probably CTE design can be only one frame, where we
can do it

WITH
 FUNCTION f1(a int) RETURNS int AS $$ .. $$ LANGUAGE plpgsql,
 FUNCTION f2(a int) RETURNS SETOF int AS $$ .. $$ LANGUAGE plpgsql,
  SELECT f1(x) FROM f2(z) LATERAL 

We can generalize WITH clause, so there SEQENCES, VIEWS, .. can be defined
for "single usage"

Regards

Pavel


>
> Pavel
>
>
>
>>
>> Greetings,
>>
>> Andres Freund
>>
>> --
>>  Andres Freund http://www.2ndQuadrant.com/
>>  PostgreSQL Development, 24x7 Support, Training & Services
>>
>
>


Re: [HACKERS] Anonymous code block with parameters

2014-09-18 Thread Pavel Stehule
2014-09-18 13:53 GMT+02:00 Andres Freund :

> On 2014-09-18 13:51:56 +0200, Pavel Stehule wrote:
> > 2014-09-18 13:48 GMT+02:00 Andres Freund :
> >
> > > On 2014-09-18 13:44:47 +0200, Pavel Stehule wrote:
> > > Isn't being able to do this on a standby a fundamental enough
> advantage?
> > > Being significantly cheaper? Needing fewer roundtrips?
> > >
> >
> > no, I don't need more. My opinion is, so this proposal has no real
> benefit,
> > but will do implement redundant functionality.
>
> FFS: What's redundant about being able to do this on a standby?
>

Is it solution for standby? It is necessary? You can have a functions on
master.

Is not higher missfeature temporary tables on stanby?

again: I am not against to DO paramaterization. I am against to implement
DO with complexity like functions. If we have a problem with standby, then
we have to fix it correctly. There is a issue with temp tables, temp
sequences, temp functions.

Pavel



>
> Greetings,
>
> Andres Freund
>
> --
>  Andres Freund http://www.2ndQuadrant.com/
>  PostgreSQL Development, 24x7 Support, Training & Services
>


Re: [HACKERS] Anonymous code block with parameters

2014-09-18 Thread Andres Freund
On 2014-09-18 13:51:56 +0200, Pavel Stehule wrote:
> 2014-09-18 13:48 GMT+02:00 Andres Freund :
> 
> > On 2014-09-18 13:44:47 +0200, Pavel Stehule wrote:
> > Isn't being able to do this on a standby a fundamental enough advantage?
> > Being significantly cheaper? Needing fewer roundtrips?
> >
> 
> no, I don't need more. My opinion is, so this proposal has no real benefit,
> but will do implement redundant functionality.

FFS: What's redundant about being able to do this on a standby?

Greetings,

Andres Freund

-- 
 Andres Freund 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] Anonymous code block with parameters

2014-09-18 Thread Pavel Stehule
2014-09-18 13:48 GMT+02:00 Andres Freund :

> On 2014-09-18 13:44:47 +0200, Pavel Stehule wrote:
> > 2014-09-18 13:40 GMT+02:00 Andres Freund :
> >
> > > On 2014-09-17 22:17:22 +0200, Pavel Stehule wrote:
> > > > 2014-09-17 22:07 GMT+02:00 Vik Fearing :
> > > I fail to see why that is so much preferrable for you to passing
> > > parameter to DO?
> >
> >
> > > 1) You need to think about unique names for functions
> > > 2) Doesn't work on HOT STANDBYs
> > > 3) Causes noticeable amount of catalog bloat
> > > 4) Is about a magnitude or two more expensive
> > >
> >
> > 1. I am not against simple DO, what doesn't substitute functions
> >
> > 2. When DO have to substitute functions, then I don't see a benefits
> >
> > Show me real use case please?
>
> Did you read what I wrote above? I'm sure you can rephrase them to be
> more 'use case' like yourself.
>
> Isn't being able to do this on a standby a fundamental enough advantage?
> Being significantly cheaper? Needing fewer roundtrips?
>

no, I don't need more. My opinion is, so this proposal has no real benefit,
but will do implement redundant functionality.

Regards

Pavel


>
> Greetings,
>
> Andres Freund
>
> --
>  Andres Freund http://www.2ndQuadrant.com/
>  PostgreSQL Development, 24x7 Support, Training & Services
>


Re: [HACKERS] Anonymous code block with parameters

2014-09-18 Thread Marko Tiikkaja

On 9/18/14 1:35 PM, Martijn van Oosterhout wrote:

On Wed, Sep 17, 2014 at 10:17:22PM +0200, Pavel Stehule wrote:

Because you still have to do

 SELECT pg_temp.my_temp_function(blah);

to execute it.



this problem should be solvable. I can to use a temporary tables without
using pg_temp schema.


Umm, IIRC it used to work that way but was changed to work like this.
IIRC the reason was that anyone can create functions in the temp
tablespace and thus hijack other functions that more priviledged
functions might call.


The same argument applies to temporary tables *already*.  Consider:

=# create function oops() returns void as $$
$# begin insert into foo default values; end $$ language plpgsql
-# security definer;
CREATE FUNCTION
=# grant execute on function oops() to peasant;
GRANT

Then peasant does:

=> create temporary table foo();
CREATE TABLE
=> create function pg_temp.now_im_superuser() returns trigger as $$
$> begin raise notice '%', pg_read_file('pg_hba.conf'); return new; end
$> $$ language plpgsql;
CREATE FUNCTION
=> create trigger malicious before insert on pg_temp.foo
-> execute procedure pg_temp.now_im_superuser();
CREATE TRIGGER
=> select oops();
NOTICE: 

Personally, I think that if we're going to do something, we should be 
*hiding* temporary stuff from search_path, not bringing it more visible. 
 Having to either prefix everything with the schema name or set 
search_path for every SECURITY DEFINER function is a major PITA.



.marko


--
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] Anonymous code block with parameters

2014-09-18 Thread Andres Freund
On 2014-09-18 13:44:47 +0200, Pavel Stehule wrote:
> 2014-09-18 13:40 GMT+02:00 Andres Freund :
> 
> > On 2014-09-17 22:17:22 +0200, Pavel Stehule wrote:
> > > 2014-09-17 22:07 GMT+02:00 Vik Fearing :
> > I fail to see why that is so much preferrable for you to passing
> > parameter to DO?
> 
> 
> > 1) You need to think about unique names for functions
> > 2) Doesn't work on HOT STANDBYs
> > 3) Causes noticeable amount of catalog bloat
> > 4) Is about a magnitude or two more expensive
> >
> 
> 1. I am not against simple DO, what doesn't substitute functions
> 
> 2. When DO have to substitute functions, then I don't see a benefits
> 
> Show me real use case please?

Did you read what I wrote above? I'm sure you can rephrase them to be
more 'use case' like yourself.

Isn't being able to do this on a standby a fundamental enough advantage?
Being significantly cheaper? Needing fewer roundtrips?

Greetings,

Andres Freund

-- 
 Andres Freund 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] Collations and Replication; Next Steps

2014-09-18 Thread Martijn van Oosterhout
On Wed, Sep 17, 2014 at 03:57:38PM +0100, Greg Stark wrote:
> Then there's the concern that ICU is a *huge* dependency. ICU is
> itself larger than the entire Postgres install. It's a big burden on
> users to have to install and configure a second collation library in
> addition to the system library and a complete non-starter for embedded
> systems or low-memory systems.

$ apt-cache show libicu52|grep Installed-Size
Installed-Size: 27283

That's 27MB or less than 2 WAL files. Or about 4 times the template
database, or which 3 are created during install and the first user
database will be a fourth.

The installed size of Postgres is 11MB not including any of the
libraries it already depends on.

Yes, it's not a small library but lets not get carried away.

And if it's optional then low memory systems can configure it out.

As for configuration, ICU doesn't require configuration just like glibc
doesn't require configuration.

Mvg,
-- 
Martijn van Oosterhout  http://svana.org/kleptog/
> He who writes carelessly confesses thereby at the very outset that he does
> not attach much importance to his own thoughts.
   -- Arthur Schopenhauer


signature.asc
Description: Digital signature


Re: [HACKERS] Anonymous code block with parameters

2014-09-18 Thread Pavel Stehule
2014-09-18 13:40 GMT+02:00 Andres Freund :

> On 2014-09-17 22:17:22 +0200, Pavel Stehule wrote:
> > 2014-09-17 22:07 GMT+02:00 Vik Fearing :
> >
> > > On 09/16/2014 10:09 AM, Heikki Linnakangas wrote:
> > > > On 09/16/2014 10:57 AM, Craig Ringer wrote:
> > > >> On 09/16/2014 03:15 PM, Pavel Stehule wrote:
> > > >>
> > > >>> Why we don't introduce a temporary functions instead?
> > > >>
> > > >> I think that'd be a lot cleaner and simpler. It's something I've
> > > >> frequently wanted, and as Hekki points out it's already possible by
> > > >> creating the function in pg_temp, there just isn't the syntax sugar
> for
> > > >> "CREATE TEMPORARY FUNCTION".
> > > >>
> > > >> So why not just add "CREATE TEMPORARY FUNCTION"?
> > > >
> > > > Sure, why not.
> > >
> > > Because you still have to do
> > >
> > > SELECT pg_temp.my_temp_function(blah);
> > >
> > > to execute it.
> > >
> >
> > this problem should be solvable. I can to use a temporary tables without
> > using pg_temp schema.
>
> I fail to see why that is so much preferrable for you to passing
> parameter to DO?


> 1) You need to think about unique names for functions
> 2) Doesn't work on HOT STANDBYs
> 3) Causes noticeable amount of catalog bloat
> 4) Is about a magnitude or two more expensive
>

1. I am not against simple DO, what doesn't substitute functions

2. When DO have to substitute functions, then I don't see a benefits

Show me real use case please?

Pavel


>
> So yes, TEMPORARY FUNCTION would be helpful. But it's simply a different
> feature.
>
> Greetings,
>
> Andres Freund
>
> --
>  Andres Freund http://www.2ndQuadrant.com/
>  PostgreSQL Development, 24x7 Support, Training & Services
>


Re: [HACKERS] Anonymous code block with parameters

2014-09-18 Thread Andres Freund
On 2014-09-17 22:17:22 +0200, Pavel Stehule wrote:
> 2014-09-17 22:07 GMT+02:00 Vik Fearing :
> 
> > On 09/16/2014 10:09 AM, Heikki Linnakangas wrote:
> > > On 09/16/2014 10:57 AM, Craig Ringer wrote:
> > >> On 09/16/2014 03:15 PM, Pavel Stehule wrote:
> > >>
> > >>> Why we don't introduce a temporary functions instead?
> > >>
> > >> I think that'd be a lot cleaner and simpler. It's something I've
> > >> frequently wanted, and as Hekki points out it's already possible by
> > >> creating the function in pg_temp, there just isn't the syntax sugar for
> > >> "CREATE TEMPORARY FUNCTION".
> > >>
> > >> So why not just add "CREATE TEMPORARY FUNCTION"?
> > >
> > > Sure, why not.
> >
> > Because you still have to do
> >
> > SELECT pg_temp.my_temp_function(blah);
> >
> > to execute it.
> >
> 
> this problem should be solvable. I can to use a temporary tables without
> using pg_temp schema.

I fail to see why that is so much preferrable for you to passing
parameter to DO?

1) You need to think about unique names for functions
2) Doesn't work on HOT STANDBYs
3) Causes noticeable amount of catalog bloat
4) Is about a magnitude or two more expensive

So yes, TEMPORARY FUNCTION would be helpful. But it's simply a different
feature.

Greetings,

Andres Freund

-- 
 Andres Freund 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] Anonymous code block with parameters

2014-09-18 Thread Martijn van Oosterhout
On Wed, Sep 17, 2014 at 10:17:22PM +0200, Pavel Stehule wrote:
> > Because you still have to do
> >
> > SELECT pg_temp.my_temp_function(blah);
> >
> > to execute it.
> >
> 
> this problem should be solvable. I can to use a temporary tables without
> using pg_temp schema.

Umm, IIRC it used to work that way but was changed to work like this.
IIRC the reason was that anyone can create functions in the temp
tablespace and thus hijack other functions that more priviledged
functions might call.

Or something like that. I think it was even a CVE.

Have a nice dat,
-- 
Martijn van Oosterhout  http://svana.org/kleptog/
> He who writes carelessly confesses thereby at the very outset that he does
> not attach much importance to his own thoughts.
   -- Arthur Schopenhauer


signature.asc
Description: Digital signature


Re: [HACKERS] Collations and Replication; Next Steps

2014-09-18 Thread Martijn van Oosterhout
On Thu, Sep 18, 2014 at 01:35:10PM +0900, Tatsuo Ishii wrote:
> In my understanding PostgreSQL's manual MUST include the ICU license
> term (this is not a problem).  What I am not so sure is, any software
> uses PostgreSQL also MUST include the ICU license or not. If yes, I
> think this is surely a problem.

Only if we're thinking of distributing it. If the user gets ICU from
their distribution then there is no need to list the licence (just like
we don't need to mention the licence of glibc).  We only need link
against it, not distribute it.

Have a nice day,
-- 
Martijn van Oosterhout  http://svana.org/kleptog/
> He who writes carelessly confesses thereby at the very outset that he does
> not attach much importance to his own thoughts.
   -- Arthur Schopenhauer


signature.asc
Description: Digital signature


Re: [HACKERS] proposal (9.5) : psql unicode border line styles

2014-09-18 Thread Pavel Stehule
2014-09-18 12:51 GMT+02:00 Andres Freund :

> On 2014-09-18 12:47:25 +0200, Pavel Stehule wrote:
> > - output = PageOutput(81, pager);
> > + output = PageOutput(87, pager);
> >
> >   fprintf(output, _("List of specially treated variables.\n"));
> >
> > @@ -364,6 +364,10 @@ helpVariables(unsigned short int pager)
> >" column
> width of left aligned data type in latex format\n"));
> >   fprintf(output, _("  title  set the table title for
> any subsequently printed tables\n"));
> >   fprintf(output, _("  tuples_onlyif set, only actual table
> data is shown\n"));
> > + fprintf(output, _("  unicode_border_linestyle\n"));
> > + fprintf(output, _("  unicode_column_linestyle\n"));
> > + fprintf(output, _("  unicode_header_linestyle\n"
> > +  " set the
> style of unicode line drawing [single, double]\n"));
> >
> >   fprintf(output, _("\nEnvironment variables:\n"));
> >   fprintf(output, _("Usage:\n"));
>
> Either the current line count is wrong, or you added the wrong number of
> new lines to PageOutput(). Your patch only adds four \n, while you
> increased from 81 to 87.
>

true, sorry, I have a different wording in first design

fixed

Regards

Pavel


>
> Greetings,
>
> Andres Freund
>
> --
>  Andres Freund http://www.2ndQuadrant.com/
>  PostgreSQL Development, 24x7 Support, Training & Services
>
commit aec4a03babdbed49d80253112b0d31108761e860
Author: Pavel Stehule 
Date:   Thu Sep 18 12:45:27 2014 +0200

initial

diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index 6035a77..d323a14 100644
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -305,7 +305,7 @@ helpVariables(unsigned short int pager)
 {
 	FILE	   *output;
 
-	output = PageOutput(81, pager);
+	output = PageOutput(85, pager);
 
 	fprintf(output, _("List of specially treated variables.\n"));
 
@@ -364,6 +364,10 @@ helpVariables(unsigned short int pager)
 	 " column width of left aligned data type in latex format\n"));
 	fprintf(output, _("  title  set the table title for any subsequently printed tables\n"));
 	fprintf(output, _("  tuples_onlyif set, only actual table data is shown\n"));
+	fprintf(output, _("  unicode_border_linestyle\n"));
+	fprintf(output, _("  unicode_column_linestyle\n"));
+	fprintf(output, _("  unicode_header_linestyle\n"
+	 " set the style of unicode line drawing [single, double]\n"));
 
 	fprintf(output, _("\nEnvironment variables:\n"));
 	fprintf(output, _("Usage:\n"));

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


Re: [HACKERS] proposal (9.5) : psql unicode border line styles

2014-09-18 Thread Andres Freund
On 2014-09-18 12:47:25 +0200, Pavel Stehule wrote:
> - output = PageOutput(81, pager);
> + output = PageOutput(87, pager);
>  
>   fprintf(output, _("List of specially treated variables.\n"));
>  
> @@ -364,6 +364,10 @@ helpVariables(unsigned short int pager)
>" column width of 
> left aligned data type in latex format\n"));
>   fprintf(output, _("  title  set the table title for any 
> subsequently printed tables\n"));
>   fprintf(output, _("  tuples_onlyif set, only actual table data 
> is shown\n"));
> + fprintf(output, _("  unicode_border_linestyle\n"));
> + fprintf(output, _("  unicode_column_linestyle\n"));
> + fprintf(output, _("  unicode_header_linestyle\n"
> +  " set the style of 
> unicode line drawing [single, double]\n"));
>  
>   fprintf(output, _("\nEnvironment variables:\n"));
>   fprintf(output, _("Usage:\n"));

Either the current line count is wrong, or you added the wrong number of
new lines to PageOutput(). Your patch only adds four \n, while you
increased from 81 to 87.

Greetings,

Andres Freund

-- 
 Andres Freund 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] proposal (9.5) : psql unicode border line styles

2014-09-18 Thread Pavel Stehule
Hi Stephen

I forgot to fix new enhanced pset help

fix attached

Regards

Pavel



2014-09-12 18:15 GMT+02:00 Pavel Stehule :

>
>
> 2014-09-12 18:09 GMT+02:00 Stephen Frost :
>
>> * Stephen Frost (sfr...@snowman.net) wrote:
>> > * Pavel Stehule (pavel.steh...@gmail.com) wrote:
>> > > I though about it, and we cannot to drop it now. These symbols are
>> > > necessary, because we don't support line between rows.
>> > >
>> > > I am thinking so 05 patch should be final
>> >
>> > Ok.  I'm going to play with it a bit more but barring issues, should get
>> > it committed today.
>>
>> Alright, pushed with some additional cleanup, and also cleaned up the
>> border documentation a bit.
>>
>
> Thank you very much
>
> Regards
>
> Pavel
>
>
>>
>> Thanks!
>>
>> Stephen
>>
>
>
commit aec4a03babdbed49d80253112b0d31108761e860
Author: Pavel Stehule 
Date:   Thu Sep 18 12:45:27 2014 +0200

initial

diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index 6035a77..d323a14 100644
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -305,7 +305,7 @@ helpVariables(unsigned short int pager)
 {
 	FILE	   *output;
 
-	output = PageOutput(81, pager);
+	output = PageOutput(87, pager);
 
 	fprintf(output, _("List of specially treated variables.\n"));
 
@@ -364,6 +364,10 @@ helpVariables(unsigned short int pager)
 	 " column width of left aligned data type in latex format\n"));
 	fprintf(output, _("  title  set the table title for any subsequently printed tables\n"));
 	fprintf(output, _("  tuples_onlyif set, only actual table data is shown\n"));
+	fprintf(output, _("  unicode_border_linestyle\n"));
+	fprintf(output, _("  unicode_column_linestyle\n"));
+	fprintf(output, _("  unicode_header_linestyle\n"
+	 " set the style of unicode line drawing [single, double]\n"));
 
 	fprintf(output, _("\nEnvironment variables:\n"));
 	fprintf(output, _("Usage:\n"));

-- 
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] [TODO] Process pg_hba.conf keywords as case-insensitive

2014-09-18 Thread Kyotaro HORIGUCHI
Hi, This is revised patch including document.

I confused three identifiers to be compared, names in the
catalog, those in pg_hba lines and those given from the client
under connecting. This patch concerns the comparison between
pg_hba and client names.

Finally all the additional pg_strcasecmp() or whole catalog
scanning are eliminated. This version works as following.

Tokenize every hba tokens and categorize having two attributes,

   One is whether the case is preserved or not. Case of a word is
   preserved in the returned token if the word is enclosed with
   double quotes.

   Another is token type, Leading bare '+' indicates the token is
   a group name, and '@' indicates file inclusion. The string in
   returned token is stripped of the special characters.

   A double quoted region which does not begin at the beginning
   of the word was handled in its own way from before this
   change. I don't know it is right or not. (ho"r""i"guti stored
   as hor"iguti by the orignal next_token() and it is not
   changed)

Matching names are performed as following,

   Tokens corrensponding to keywords should be 'normal' ones (not
   a group name or file inclusion) and should not be
   case-preserved ones, which were enclosed by double quotes. The
   tokens are lowercased so token_is_keyword() macro compares
   them by strcmp().

   Database name and user name should be 'normal' tokens and the
   cases of the names are preserved or not according to the
   notaion in hba line so token_matches() compares them with the
   name given from client by strcmp().


The patch size is far reduced from the previous version.


At Wed, 10 Sep 2014 11:32:22 +0200, Florian Pflug  wrote in 
<7d70ee06-1e80-44d6-9428-5f60ad796...@phlo.org>
> So foo, Foo and FOO would all match the user called ,
> but "Foo" would match the user called , and "FOO" the
> user called .

This patch does so.

> An unquoted "+" would cause whatever follows it to be interpreted
> as a group name, whereas a quoted "+" would simply become part of
> the user name (or group name, if there's an additional unquoted
> "+" before it).
> So +foo would refer to the group , +"FOO" to the group ,
> and +"+A" to the group <+A>.

I think this behaves so.

> I haven't checked if such an approach would be sufficiently
> backwards-compatible, though.

One obveous breaking which affects the existing sane pg_hba.conf
is that db and user names not surrounded by double quotes became
to match the lowercased names, not the original name containing
uppercase characters. But this is just what this patch intended.

I think all behaviors for other cases appear in existing
pg_hba.conf are unchanged including the behaviors for string
consists of single character '+' or '@'.

# '+' is treated as a group name '' and '@' is treated as a
# user/db name '@' but they seems meanless..

Any suggestions?

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
>From b02cea3ead352a198f341c0f2a9f6ab93f439077 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi 
Date: Thu, 18 Sep 2014 17:06:21 +0900
Subject: [PATCH 2/2] Make pg_hba.conf case insensitive.

---
 src/backend/libpq/hba.c |  101 ++
 1 files changed, 83 insertions(+), 18 deletions(-)

diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index 84da823..e4b1635 100644
--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -60,9 +60,18 @@ typedef struct check_network_data
 	bool		result;			/* set to true if match */
 } check_network_data;
 
+typedef enum TokenType
+{
+	NORMAL,
+	GROUP_NAME,			/* this token had leading '+' */
+	FILE_INCLUSION,		/* this token had leading '@' */
+} TokenType;
 
-#define token_is_keyword(t, k)	(!t->quoted && strcmp(t->string, k) == 0)
-#define token_matches(t, k)  (strcmp(t->string, k) == 0)
+#define token_is_keyword(tk, kw)	\
+	((tk)->type == NORMAL && !(tk)->case_preserved &&	\
+	 (strcmp((tk)->string, (kw)) == 0))
+#define token_matches(t, k)	\
+	((t)->type == NORMAL && (strcmp((t)->string, (k)) == 0))
 
 /*
  * A single string token lexed from the HBA config file, together with whether
@@ -71,7 +80,8 @@ typedef struct check_network_data
 typedef struct HbaToken
 {
 	char	   *string;
-	bool		quoted;
+	TokenType	type;
+	bool		case_preserved;
 } HbaToken;
 
 /*
@@ -123,8 +133,14 @@ pg_isblank(const char c)
  * the first character.  (We use that to prevent "@x" from being treated
  * as a file inclusion request.  Note that @"x" should be so treated;
  * we want to allow that to support embedded spaces in file paths.)
+ * type is one of NORMAL, GROUP_NAME, FILE_INCLUSION. GROUP_NAME is set if the
+ * token is prefix by '+', FILE_INCLUSION if prefixed by '@', NORMAL
+ * otherwise.
  * We set *terminating_comma to indicate whether the token is terminated by a
  * comma (which is not returned.)
+ * case_preserved is set if the token's case of every character is preserved
+ * when the whole word is enclosed by double quotes. Elsewise returned to

Re: [HACKERS] New developer TODO suggestions

2014-09-18 Thread Andres Freund
On 2014-09-18 17:30:38 +0800, Craig Ringer wrote:
> On 07/29/2014 10:43 PM, Bruce Momjian wrote:
> >> > * When a user tries to run "psql -f some_custom_format_backup", detect
> >> > this and emit a useful error message. Ditto stdin.
> >
> > Uh, good idea, but can we really do that in psql?
> 
> Where stdin is a file, or an explicit -f is given, then yes.
> 
> Just look for PGDMP as the first five bytes and complain.
> 
> To do it more generally we'd need to look at each statement and see if
> it seems to begin with PGDMP. I'm not sure that's worth it; handling the
> simple cases of
> 
> psql -f mydb.dump
> 
> and
> 
> psql < mydb.dump
> 
> would be quite sufficient.
> 
> I'm not 100% sure if we can easily differentiate the second case from
> "pg_dump | psql"; I know there's isatty(...) to test if stdin is a
> terminal, but I'm not sure how easy/possible it is to tell if it's a
> file not a pipe.

I don't think we need to make any discinction between psql -f mydb.dump,
psql < mydb.dump, and whatever | psql. Just check, when noninteractively
reading the first line in mainloop.c:MainLoop(), whether it starts with
the magic header. That'd also trigger the warning on \i pg_restore_file,
but that's hardly a problem.

> pg_restore already knows to tell you to use psql if it sees an SQL file
> as input. Having something similar for pg_dump would be really useful.

Agreed.

We could additionally write out a hint whenever a directory is fed to
psql -f that psql can't be used to read directory type dumps.

Greetings,

Andres Freund

-- 
 Andres Freund 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] New developer TODO suggestions

2014-09-18 Thread Craig Ringer
On 07/29/2014 10:43 PM, Bruce Momjian wrote:
>> > * When a user tries to run "psql -f some_custom_format_backup", detect
>> > this and emit a useful error message. Ditto stdin.
>
> Uh, good idea, but can we really do that in psql?

Where stdin is a file, or an explicit -f is given, then yes.

Just look for PGDMP as the first five bytes and complain.

To do it more generally we'd need to look at each statement and see if
it seems to begin with PGDMP. I'm not sure that's worth it; handling the
simple cases of

psql -f mydb.dump

and

psql < mydb.dump

would be quite sufficient.

I'm not 100% sure if we can easily differentiate the second case from
"pg_dump | psql"; I know there's isatty(...) to test if stdin is a
terminal, but I'm not sure how easy/possible it is to tell if it's a
file not a pipe.

pg_restore already knows to tell you to use psql if it sees an SQL file
as input. Having something similar for pg_dump would be really useful.

-- 
 Craig Ringer   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] END_OF_RECOVERY shutdowns and ResetUnloggedRelations()

2014-09-18 Thread Andres Freund
On 2014-09-12 14:44:48 -0400, Robert Haas wrote:
> On Fri, Sep 12, 2014 at 7:22 AM, Andres Freund  wrote:
> > What I like even less is that pg_control is actually marked as
> > DB_SHUTDOWNED due to END_OF_RECOVERY. That's just plain wrong. Obviously
> > the database was *NOT* shutdown peacefully. I don't see active bugs due
> > it besides this, but I think it's likely to either have or create futher
> > ones.
> 
> I agree.  The database clearly isn't shut down at end of recovery;
> it's still active and we're still doing things to it.  If we crash at
> that point, we need to go back into recovery on restart.  This seems
> open and shut, but maybe I'm missing something.  Why shouldn't we fix
> *that*?

Well, I think we might want to do both. There doesn't seem to be a
reason to *not* do the ResetUnloggedRelation(UNLOGGED_RELATION_INIT)
around the ShutdownWalRcv(). That seems much closer where it, for me,
logically belongs. And it'd fix the concrete problem.

For DB_SHUTDOWNED (is that actually a word? Looks like it could be from
me...) the case isn't that clear:

If you start a node after a crash and stop it as soon as it finished, it
doesn't need recovery again. Similar if a node is promoted and doesn't
use fast promotion or a older release. Now, I think this is a pretty
dubious benefit. But I'm not sure it's wise to change it in the back
branches.

> With regard to your second email, I agree that
> ResetUnloggedRelation(UNLOGGED_RELATION_INIT) needs to issue fsyncs.

Good.

The topic reminds me of the fact that I actually think at the very least
pg_xlog/ and pg_control needs the same treatment. Consider the following
sequence:
1) postgres writes loads of stuff. Lots of it to *not* fsynced WAL
   segments
2) postgres crashes and crash recovery happens. Replays *not* fsynced
   WAL
3) the OS crashes
4) Bad. We now might hava pg_control with a minRecovery that's *later*
   than some potentially unsynced WAL segments

I think the easiest would be to just fsync() the entire data directory
at the start when ControlFile->state !=
DB_SHUTDOWNED/DB_SHUTDOWNED_IN_RECOVERY

Note that that's independent of the fsync for unlogged relations.

Greetings,

Andres Freund

-- 
 Andres Freund 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] Options OUTPUT_PLUGIN_* controlling format are confusing (Was: Misleading error message in logical decoding)

2014-09-18 Thread Andres Freund
Hi,

On 2014-09-18 02:14:48 -0500, Michael Paquier wrote:
> As there has been a dump of CATALOG_VERSION_NO on REL9_4_STABLE
> recently, I am coming back to the options OUTPUT_PLUGIN_* that are
> rather confusing for developers of decoder plugins. In short, when
> extracting changes from a replication slot, a decoder plugin is free
> to set one option that influences the output that is fetched by a
> logical receiver: OUTPUT_PLUGIN_TEXTUAL_OUTPUT or
> OUTPUT_PLUGIN_BINARY_OUTPUT. The interesting point about this format
> option is that it does not directly influence the changes generated by
> an output plugin: its value only has effect on the set of functions
> pg_logical_[get|peek]_[binary_|]changes that can be used by a
> non-replication connection to get individual changes from a repslot.
> 
> So a receiver fetching changes using PQgetCopyData is not really
> impacted by this format option. Even better it is even possible to use
> a custom option that is part of output_plugin_options to switch the
> OUTPUT_PLUGIN_* value (cf option force-binary in
> contrib/test_decoding).
> 
> My point is: logical decoding is presenting in its infrastructure API
> a format option that could be added as a custom option in a decoder
> plugin. Isn't that orthogonal? To illustrate this argument here is an
> example: we could remove force-binary in test_decoding and replace it
> by a option that allows the user to directly choose the output format,
> like format=[binary|textual] and do the conversion in the plugin. In
> the same manner, the functions pg_logical_[get|peek]_binary_changes
> are equivalent to their cousin pg_logical_[get|peek]_changes casted to
> bytea.

The point is that operating with byteas on SQL level is freaking
painful.

> I am raising this point before the 9.4 ship sails, thinking long-term
> and to faciliate the maintenance of existing code. Attached is a patch
> that simplifies the current logical decoding API regarding that.

Sorry, -1.

Improving the docs here is on my roadmap, but I don't see the benefit of
this.

Greetings,

Andres Freund

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


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


[HACKERS] Options OUTPUT_PLUGIN_* controlling format are confusing (Was: Misleading error message in logical decoding)

2014-09-18 Thread Michael Paquier
Hi all,

As there has been a dump of CATALOG_VERSION_NO on REL9_4_STABLE
recently, I am coming back to the options OUTPUT_PLUGIN_* that are
rather confusing for developers of decoder plugins. In short, when
extracting changes from a replication slot, a decoder plugin is free
to set one option that influences the output that is fetched by a
logical receiver: OUTPUT_PLUGIN_TEXTUAL_OUTPUT or
OUTPUT_PLUGIN_BINARY_OUTPUT. The interesting point about this format
option is that it does not directly influence the changes generated by
an output plugin: its value only has effect on the set of functions
pg_logical_[get|peek]_[binary_|]changes that can be used by a
non-replication connection to get individual changes from a repslot.

So a receiver fetching changes using PQgetCopyData is not really
impacted by this format option. Even better it is even possible to use
a custom option that is part of output_plugin_options to switch the
OUTPUT_PLUGIN_* value (cf option force-binary in
contrib/test_decoding).

My point is: logical decoding is presenting in its infrastructure API
a format option that could be added as a custom option in a decoder
plugin. Isn't that orthogonal? To illustrate this argument here is an
example: we could remove force-binary in test_decoding and replace it
by a option that allows the user to directly choose the output format,
like format=[binary|textual] and do the conversion in the plugin. In
the same manner, the functions pg_logical_[get|peek]_binary_changes
are equivalent to their cousin pg_logical_[get|peek]_changes casted to
bytea.

I am raising this point before the 9.4 ship sails, thinking long-term
and to faciliate the maintenance of existing code. Attached is a patch
that simplifies the current logical decoding API regarding that.
Regards,
-- 
Michael
From dea334ff685c9587b9a28fe4feb324c4dcc870c2 Mon Sep 17 00:00:00 2001
From: Michael Paquier 
Date: Thu, 18 Sep 2014 01:53:22 -0500
Subject: [PATCH] Remove OUTPUT_PLUGIN_* from existing logical decoding APIs

This is confusing for users as this option only influences the output
of get/peek options that the logical decoding facility is offering. Also,
there exist other possibilities for the user to generate binary output:
- Use a bytea cast on the textual get/peek functions
- integrate an option within the plugin that generates binary output
at will.

In short, the concept of options should remain exclusively in
output_plugin_options and not elsewhere.
---
 contrib/test_decoding/Makefile |  2 +-
 contrib/test_decoding/sql/binary.sql   | 14 ---
 contrib/test_decoding/test_decoding.c  | 21 +-
 doc/src/sgml/func.sgml | 36 -
 doc/src/sgml/logicaldecoding.sgml  | 13 +--
 src/backend/catalog/system_views.sql   | 16 
 src/backend/replication/logical/logical.c  | 11 +++---
 src/backend/replication/logical/logicalfuncs.c | 53 --
 src/include/catalog/pg_proc.h  |  6 +--
 src/include/replication/logical.h  |  1 -
 src/include/replication/logicalfuncs.h |  2 -
 src/include/replication/output_plugin.h| 15 
 src/tools/pgindent/typedefs.list   |  2 -
 13 files changed, 17 insertions(+), 175 deletions(-)
 delete mode 100644 contrib/test_decoding/sql/binary.sql

diff --git a/contrib/test_decoding/Makefile b/contrib/test_decoding/Makefile
index d7f32c3..9f48947 100644
--- a/contrib/test_decoding/Makefile
+++ b/contrib/test_decoding/Makefile
@@ -37,7 +37,7 @@ submake-isolation:
 submake-test_decoding:
 	$(MAKE) -C $(top_builddir)/contrib/test_decoding
 
-REGRESSCHECKS=ddl rewrite toast permissions decoding_in_xact binary prepared
+REGRESSCHECKS=ddl rewrite toast permissions decoding_in_xact prepared
 
 regresscheck: all | submake-regress submake-test_decoding
 	$(MKDIR_P) regression_output
diff --git a/contrib/test_decoding/sql/binary.sql b/contrib/test_decoding/sql/binary.sql
deleted file mode 100644
index 619f00b..000
--- a/contrib/test_decoding/sql/binary.sql
+++ /dev/null
@@ -1,14 +0,0 @@
--- predictability
-SET synchronous_commit = on;
-
-SELECT 'init' FROM pg_create_logical_replication_slot('regression_slot', 'test_decoding');
--- succeeds, textual plugin, textual consumer
-SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'force-binary', '0');
--- fails, binary plugin, textual consumer
-SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'force-binary', '1');
--- succeeds, textual plugin, binary consumer
-SELECT data FROM pg_logical_slot_get_binary_changes('regression_slot', NULL, NULL, 'force-binary', '0');
--- succeeds, binary plugin, binary consumer
-SELECT data FROM pg_logical_slot_get_binary_changes('regression_slot', NULL, NULL, 'force-binary', '1');
-
-SELECT 'init' FROM pg_drop_replication_slot('regression_slot');
diff --git a/contrib/test_decoding/test_decoding.c b/contrib/test_decoding