[HACKERS] possible enhancing of UPDATE syntax?

[Pavel Stehule]

Hello

we support syntax for INSERT and DELETE statements, that allows a simple
triggers for these statements.

CREATE FUNCTION trg_insert()
RETURNS trigger AS $$
BEGIN
  INSERT INTO target_tbl VALUES(NEW.*)
  RETURN NULL;
END;
$$ LANGUAGE plpgsql;

* is not allowed in DELETE, but we can use a virtual column

CREATE FUNCTION trg_delete()
RETURNS trigger AS $$
BEGIN
  DELETE FROM target_tbl WHERE target_tbl = OLD;
  RETURN NULL;
END;
$$ LANGUAGE plpgsql;

Same functionality is missing for UPDATE, although we support (targetlist)
= (ROW) syntax

So should be nice and consistent with previous behave

UPDATE target_tbl SET (target_tbl.*) = (NEW) WHERE target_tbl = OLD;

What do you think about it?

Regards

Pavel

Re: [HACKERS] undefined symbol: PQescapeLiteral

[amulsul]

>You're linking against a pre-9.0 copy of libpq.so.

Thanks for help :)

I got my mistake .


Regards,
Amul Sul



--
View this message in context: 
http://postgresql.1045698.n5.nabble.com/undefined-symbol-PQescapeLiteral-tp5767578p5767994.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.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] Parsing bool type value

[Sawada Masahiko]

Hi all,

Taking a look at PostgreSQL HEAD today, I noticed that currently
PostgreSQL allows "of" value as bool type value.
So user can execute the following SQL.

=# SET enbale_seqscan TO of;

And I read the source code related to parsing bool value.
It compare TWO characters "off" and the setting value in
parse_bool_with_len() function.
Should we deny the "of" value as bool type value?

Regards,

---
Sawada Masahiko


parse_bool_with_len.patch
Description: Binary data

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

Re: [HACKERS] Extension Templates S03E11

[Boszormenyi Zoltan]

Hi,

2013-08-04 15:20 keltezéssel, Dimitri Fontaine írta:

Thom Brown  writes:

Could you please resubmit this without using SnapshotNow as it's no longer
supported?

Sure, sorry that I missed that, please find v12 attached.


Here's a review for this patch.

* Is the patch in a patch format which has context? (eg: context diff format)

Yes.

* Does it apply cleanly to the current git master?

No, it has one reject in src/bin/pg_dump/pg_backup_archiver.c.
It was obvious to fix, version 12a is attached.

* Does it include reasonable tests, necessary doc patches, etc?

It has extended the SQL reference nicely but the reference to
 was not obvious enough
regarding the list of control parameters.

The SQL syntax has them in allcaps, while the referenced section
has them in lower case. It's easy to miss them while just browsing
for e.g. RELOCATABLE. I had to go back twice to find the proper
part of the text.

I would like to see the control parameters documented in allcaps
in CREATE EXTENSION TEMPLATE. Then ALTER EXTENSION
TEMPLATE should reference the CREATE instead of the longer
text in . This xref can still
be there for reference in all three of CREATE/ALTER/DROP
EXTENSION TEMPLATE.

* Does the patch actually implement what it's supposed to do?

Yes.

* Do we want that?

Yes.

* Do we already have it?

No.

* Does it follow SQL spec, or the community-agreed behavior?

There's no such provision in the spec.
As far as I can tell, it follows the community-agreed behavior.

* Does it include pg_dump support (if applicable)?

Yes.

But the version check is already wrong in src/bin/pg_dump/pg_dump.c
since this patch missed 9.3.

+   /*
+* Before 9.3, there are no extension templates.
+*/
+   if (fout->remoteVersion < 90300)
+   {
+   *numExtensionTemplates = 0;
+   return NULL;
+   }
+

* Are there dangers?

I don't think so.

* Have all the bases been covered?

It seems so.

* Does the feature work as advertised?

Yes.

* Are there corner cases the author has failed to consider?

I don't know.

* Are there any assertion failures or crashes?

No.

* Does the patch slow down simple tests?

No.

* If it claims to improve performance, does it?

n/a

* Does it slow down other things?

No.

* Does it follow the project coding guidelines?

Yes.

Nitpicking. This chunk has an extra unnecessary space:

diff --git a/src/backend/catalog/Makefile b/src/backend/catalog/Makefile
index c4d3f3c..689dc37 100644
--- a/src/backend/catalog/Makefile
+++ b/src/backend/catalog/Makefile
@@ -38,6 +38,7 @@ POSTGRES_BKI_SRCS = $(addprefix 
$(top_srcdir)/src/include/catalog/,\
pg_authid.h pg_auth_members.h pg_shdepend.h pg_shdescription.h \
pg_ts_config.h pg_ts_config_map.h pg_ts_dict.h \
pg_ts_parser.h pg_ts_template.h pg_extension.h \
+pg_extension_control.h pg_extension_template.h pg_extension_uptmpl.h \
pg_foreign_data_wrapper.h pg_foreign_server.h pg_user_mapping.h \
pg_foreign_table.h \
pg_default_acl.h pg_seclabel.h pg_shseclabel.h pg_collation.h 
pg_range.h \

* Are there portability issues?

No.

* Will it work on Windows/BSD etc?

It should.

* Are the comments sufficient and accurate?

Yes.

* Does it do what it says, correctly?

According to the added regression tests, yes.

* Does it produce compiler warnings?

No.

* Can you make it crash?

No.

* Is everything done in a way that fits together coherently with other 
features/modules?

I think so.

* Are there interdependencies that can cause problems?

I don't know.

Best regards,
Zoltán Böszörményi

--
--
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt, Austria
Web: http://www.postgresql-support.de
 http://www.postgresql.at/



templates.v12a.patch.gz
Description: Unix tar archive

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

[HACKERS] PL/pgSQL PERFORM with CTE

[David E. Wheeler]

Hackers,

This seems reasonable:

david=# DO $$
david$# BEGIN
david$# WITH now AS (SELECT now())
david$# SELECT * from now;
david$# END;
david$# $$;
ERROR:  query has no destination for result data
HINT:  If you want to discard the results of a SELECT, use PERFORM instead.
CONTEXT:  PL/pgSQL function inline_code_block line 3 at SQL statement

This not so much:

david=# DO $$
david$# BEGIN
david$# WITH now AS (SELECT now())
david$# PERFORM * from now;
david$# END;
david$# $$;
ERROR:  syntax error at or near "PERFORM"
LINE 4: PERFORM * from now;
^
Parser bug in PL/pgSQL, perhaps?

Best,

David



-- 
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] PL/pgSQL PERFORM with CTE

[Pavel Stehule]

Hello


2013/8/20 David E. Wheeler 

> Hackers,
>
> This seems reasonable:
>
> david=# DO $$
> david$# BEGIN
> david$# WITH now AS (SELECT now())
> david$# SELECT * from now;
> david$# END;
> david$# $$;
> ERROR:  query has no destination for result data
> HINT:  If you want to discard the results of a SELECT, use PERFORM
> instead.
> CONTEXT:  PL/pgSQL function inline_code_block line 3 at SQL statement
>
> This not so much:
>
> david=# DO $$
> david$# BEGIN
> david$# WITH now AS (SELECT now())
> david$# PERFORM * from now;
> david$# END;
> david$# $$;
> ERROR:  syntax error at or near "PERFORM"
> LINE 4: PERFORM * from now;
> ^
> Parser bug in PL/pgSQL, perhaps?
>

no

you cannot use a PL/pgSQL statement inside SQL statement.

Regards

Pavel


>
> Best,
>
> David
>
>
>
> --
> 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] PL/pgSQL PERFORM with CTE

[David E. Wheeler]

Hi Pavel,

On Aug 20, 2013, at 2:11 PM, Pavel Stehule  wrote:

>> david=# DO $$
>> david$# BEGIN
>> david$# WITH now AS (SELECT now())
>> david$# PERFORM * from now;
>> david$# END;
>> david$# $$;
>> ERROR:  syntax error at or near "PERFORM"
>> LINE 4: PERFORM * from now;
>> ^
>> Parser bug in PL/pgSQL, perhaps?
> 
> no
> 
> you cannot use a PL/pgSQL statement inside SQL statement.

Well, there ought to be *some* way to tell PL/pgSQL to discard the result. 
Right now I am adding a variable to select into but never otherwise use. 
Inelegant, IMHO. Perhaps I’m missing some other way to do it?

If so, it would help if the hint suggesting the use of PERFORM pointed to such 
alternatives.

Best,

David



-- 
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] PL/pgSQL PERFORM with CTE

[Pavel Stehule]

2013/8/20 David E. Wheeler 

> Hi Pavel,
>
> On Aug 20, 2013, at 2:11 PM, Pavel Stehule 
> wrote:
>
> >> david=# DO $$
> >> david$# BEGIN
> >> david$# WITH now AS (SELECT now())
> >> david$# PERFORM * from now;
> >> david$# END;
> >> david$# $$;
> >> ERROR:  syntax error at or near "PERFORM"
> >> LINE 4: PERFORM * from now;
> >> ^
> >> Parser bug in PL/pgSQL, perhaps?
> >
> > no
> >
> > you cannot use a PL/pgSQL statement inside SQL statement.
>
> Well, there ought to be *some* way to tell PL/pgSQL to discard the result.
> Right now I am adding a variable to select into but never otherwise use.
> Inelegant, IMHO. Perhaps I’m missing some other way to do it?
>
> If so, it would help if the hint suggesting the use of PERFORM pointed to
> such alternatives.
>

postgres=# DO $$
 BEGIN
   PERFORM * FROM (WITH now AS (SELECT now())
  SELECT * from now) x;
 END;
$$;
DO
postgres=#

Regards

Pavel


>
> Best,
>
> David
>
>

Re: [HACKERS] PL/pgSQL PERFORM with CTE

[Andres Freund]

On 2013-08-20 14:15:55 +0200, David E. Wheeler wrote:
> Hi Pavel,
> 
> On Aug 20, 2013, at 2:11 PM, Pavel Stehule  wrote:
> 
> >> david=# DO $$
> >> david$# BEGIN
> >> david$# WITH now AS (SELECT now())
> >> david$# PERFORM * from now;
> >> david$# END;
> >> david$# $$;
> >> ERROR:  syntax error at or near "PERFORM"
> >> LINE 4: PERFORM * from now;
> >> ^
> >> Parser bug in PL/pgSQL, perhaps?
> > 
> > no
> > 
> > you cannot use a PL/pgSQL statement inside SQL statement.
> 
> Well, there ought to be *some* way to tell PL/pgSQL to discard the result. 
> Right now I am adding a variable to select into but never otherwise use. 
> Inelegant, IMHO. Perhaps I’m missing some other way to do it?
> 
> If so, it would help if the hint suggesting the use of PERFORM pointed to 
> such alternatives.

Not that that's elegant but IIRC PERFORM (WITH ...) ought to work. I
don't think the intermingled plpgsql/sql grammars allow a nice way right
now.

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] PL/pgSQL PERFORM with CTE

[Marko Tiikkaja]

On 8/20/13 2:21 PM, Pavel Stehule wrote:

2013/8/20 David E. Wheeler 

Well, there ought to be *some* way to tell PL/pgSQL to discard the result.
Right now I am adding a variable to select into but never otherwise use.
Inelegant, IMHO. Perhaps I’m missing some other way to do it?

If so, it would help if the hint suggesting the use of PERFORM pointed to
such alternatives.



postgres=# DO $$
  BEGIN
PERFORM * FROM (WITH now AS (SELECT now())
   SELECT * from now) x;
  END;
$$;
DO


.. which doesn't work if you want to use table-modifying CTEs.


Regards,
Marko Tiikkaja



--
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] PL/pgSQL PERFORM with CTE

[Pavel Stehule]

2013/8/20 Andres Freund 

> On 2013-08-20 14:15:55 +0200, David E. Wheeler wrote:
> > Hi Pavel,
> >
> > On Aug 20, 2013, at 2:11 PM, Pavel Stehule 
> wrote:
> >
> > >> david=# DO $$
> > >> david$# BEGIN
> > >> david$# WITH now AS (SELECT now())
> > >> david$# PERFORM * from now;
> > >> david$# END;
> > >> david$# $$;
> > >> ERROR:  syntax error at or near "PERFORM"
> > >> LINE 4: PERFORM * from now;
> > >> ^
> > >> Parser bug in PL/pgSQL, perhaps?
> > >
> > > no
> > >
> > > you cannot use a PL/pgSQL statement inside SQL statement.
> >
> > Well, there ought to be *some* way to tell PL/pgSQL to discard the
> result. Right now I am adding a variable to select into but never otherwise
> use. Inelegant, IMHO. Perhaps I’m missing some other way to do it?
> >
> > If so, it would help if the hint suggesting the use of PERFORM pointed
> to such alternatives.
>
> Not that that's elegant but IIRC PERFORM (WITH ...) ought to work. I
> don't think the intermingled plpgsql/sql grammars allow a nice way right
> now.
>

+1

Pavel



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

Re: [HACKERS] PL/pgSQL PERFORM with CTE

[David E. Wheeler]

On Aug 20, 2013, at 2:24 PM, Marko Tiikkaja  wrote:

>> postgres=# DO $$
>>  BEGIN
>>PERFORM * FROM (WITH now AS (SELECT now())
>>   SELECT * from now) x;
>>  END;
>> $$;
>> DO
> 
> .. which doesn't work if you want to use table-modifying CTEs.

Which, in fact, is exactly my use case (though not what I posted upthread).

Best,

David



-- 
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] PL/pgSQL PERFORM with CTE

[Pavel Stehule]

2013/8/20 David E. Wheeler 

> On Aug 20, 2013, at 2:24 PM, Marko Tiikkaja  wrote:
>
> >> postgres=# DO $$
> >>  BEGIN
> >>PERFORM * FROM (WITH now AS (SELECT now())
> >>   SELECT * from now) x;
> >>  END;
> >> $$;
> >> DO
> >
> > .. which doesn't work if you want to use table-modifying CTEs.
>
> Which, in fact, is exactly my use case (though not what I posted upthread).
>

but it works

postgres=# do $$begin with x as (select 10) insert into omega select * from
x; end;$$;
DO

Regards

Pavel



>
> Best,
>
> David
>
>

Re: [HACKERS] PL/pgSQL PERFORM with CTE

[David E. Wheeler]

On Aug 20, 2013, at 2:31 PM, Pavel Stehule  wrote:

> but it works
> 
> postgres=# do $$begin with x as (select 10) insert into omega select * from 
> x; end;$$;
> DO

But this does not:

david=# DO $$
david$# BEGIN
david$# PERFORM * FROM (
david$# WITH inserted AS (
david$# INSERT INTO foo values (1) RETURNING id
david$# ) SELECT inserted.id
david$# ) x;
david$# END;
david$# $$;
ERROR:  WITH clause containing a data-modifying statement must be at the top 
level
LINE 2: WITH inserted AS (
 ^
QUERY:  SELECT * FROM (
WITH inserted AS (
INSERT INTO foo values (1) RETURNING id
) SELECT inserted.id
) x
CONTEXT:  PL/pgSQL function inline_code_block line 3 at PERFORM

Best,

David



-- 
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] PL/pgSQL PERFORM with CTE

[Pavel Stehule]

2013/8/20 David E. Wheeler 

> On Aug 20, 2013, at 2:31 PM, Pavel Stehule 
> wrote:
>
> > but it works
> >
> > postgres=# do $$begin with x as (select 10) insert into omega select *
> from x; end;$$;
> > DO
>
> But this does not:
>
> david=# DO $$
> david$# BEGIN
> david$# PERFORM * FROM (
> david$# WITH inserted AS (
> david$# INSERT INTO foo values (1) RETURNING id
> david$# ) SELECT inserted.id
> david$# ) x;
> david$# END;
> david$# $$;
> ERROR:  WITH clause containing a data-modifying statement must be at the
> top level
> LINE 2: WITH inserted AS (
>  ^
> QUERY:  SELECT * FROM (
> WITH inserted AS (
> INSERT INTO foo values (1) RETURNING id
> ) SELECT inserted.id
> ) x
> CONTEXT:  PL/pgSQL function inline_code_block line 3 at PERFORM
>
> yes, in this context you should not use a PERFORM

PL/pgSQL protect you before useless queries - so you can use a CTE without
returned result directly or CTE with result via PERFORM statement (and in
this case it must be unmodifing CTE).

Sorry, I don't see any problem - why you return some from CTE and then you
throw this result?



> Best,
>
> David
>
>

Re: [HACKERS] PL/pgSQL PERFORM with CTE

[Merlin Moncure]

On Tue, Aug 20, 2013 at 7:25 AM, Andres Freund  wrote:
> On 2013-08-20 14:15:55 +0200, David E. Wheeler wrote:
>> Hi Pavel,
>>
>> On Aug 20, 2013, at 2:11 PM, Pavel Stehule  wrote:
>>
>> >> david=# DO $$
>> >> david$# BEGIN
>> >> david$# WITH now AS (SELECT now())
>> >> david$# PERFORM * from now;
>> >> david$# END;
>> >> david$# $$;
>> >> ERROR:  syntax error at or near "PERFORM"
>> >> LINE 4: PERFORM * from now;
>> >> ^
>> >> Parser bug in PL/pgSQL, perhaps?
>> >
>> > no
>> >
>> > you cannot use a PL/pgSQL statement inside SQL statement.
>>
>> Well, there ought to be *some* way to tell PL/pgSQL to discard the result. 
>> Right now I am adding a variable to select into but never otherwise use. 
>> Inelegant, IMHO. Perhaps I’m missing some other way to do it?
>>
>> If so, it would help if the hint suggesting the use of PERFORM pointed to 
>> such alternatives.
>
> Not that that's elegant but IIRC PERFORM (WITH ...) ought to work. I
> don't think the intermingled plpgsql/sql grammars allow a nice way right
> now.

I think the way forward is to remove the restriction such that data
returning queries must be PERFORM'd.

merlin


-- 
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] PL/pgSQL PERFORM with CTE

[Pavel Stehule]

2013/8/20 Merlin Moncure 

> On Tue, Aug 20, 2013 at 7:25 AM, Andres Freund 
> wrote:
> > On 2013-08-20 14:15:55 +0200, David E. Wheeler wrote:
> >> Hi Pavel,
> >>
> >> On Aug 20, 2013, at 2:11 PM, Pavel Stehule 
> wrote:
> >>
> >> >> david=# DO $$
> >> >> david$# BEGIN
> >> >> david$# WITH now AS (SELECT now())
> >> >> david$# PERFORM * from now;
> >> >> david$# END;
> >> >> david$# $$;
> >> >> ERROR:  syntax error at or near "PERFORM"
> >> >> LINE 4: PERFORM * from now;
> >> >> ^
> >> >> Parser bug in PL/pgSQL, perhaps?
> >> >
> >> > no
> >> >
> >> > you cannot use a PL/pgSQL statement inside SQL statement.
> >>
> >> Well, there ought to be *some* way to tell PL/pgSQL to discard the
> result. Right now I am adding a variable to select into but never otherwise
> use. Inelegant, IMHO. Perhaps I’m missing some other way to do it?
> >>
> >> If so, it would help if the hint suggesting the use of PERFORM pointed
> to such alternatives.
> >
> > Not that that's elegant but IIRC PERFORM (WITH ...) ought to work. I
> > don't think the intermingled plpgsql/sql grammars allow a nice way right
> > now.
>
> I think the way forward is to remove the restriction such that data
> returning queries must be PERFORM'd


I disagree, current rule has sense.

Pavel


>
> merlin
>

Re: [HACKERS] PL/pgSQL PERFORM with CTE

[David E. Wheeler]

On Aug 20, 2013, at 2:41 PM, Pavel Stehule  wrote:

> yes, in this context you should not use a PERFORM
> 
> PL/pgSQL protect you before useless queries - so you can use a CTE without 
> returned result directly or CTE with result via PERFORM statement (and in 
> this case it must be unmodifing CTE).
> 
> Sorry, I don't see any problem - why you return some from CTE and then you 
> throw this result?

I am passing the values returned from a CTE to a call to pg_notify(). I do not 
care to collect the output of pg_notify(), which returns VOID.

Best,

David



-- 
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] PL/pgSQL PERFORM with CTE

[Boszormenyi Zoltan]

2013-08-20 14:35 keltezéssel, David E. Wheeler írta:

On Aug 20, 2013, at 2:31 PM, Pavel Stehule  wrote:


but it works

postgres=# do $$begin with x as (select 10) insert into omega select * from x; 
end;$$;
DO

But this does not:

david=# DO $$
david$# BEGIN
david$# PERFORM * FROM (
david$# WITH inserted AS (
david$# INSERT INTO foo values (1) RETURNING id
david$# ) SELECT inserted.id
david$# ) x;
david$# END;
david$# $$;
ERROR:  WITH clause containing a data-modifying statement must be at the top 
level
LINE 2: WITH inserted AS (
  ^
QUERY:  SELECT * FROM (
 WITH inserted AS (
 INSERT INTO foo values (1) RETURNING id
 ) SELECT inserted.id
 ) x
CONTEXT:  PL/pgSQL function inline_code_block line 3 at PERFORM


This is the same error as if you put the WITH into a subquery,
which is what PERFORM does.

Proof:

SELECT * FROM (
WITH inserted AS (
INSERT INTO foo values (1) RETURNING id
) SELECT inserted.id
) x;


Best regards,
Zoltán Böszörményi

--
--
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt, Austria
Web: http://www.postgresql-support.de
 http://www.postgresql.at/



--
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] PL/pgSQL PERFORM with CTE

[David E. Wheeler]

On Aug 20, 2013, at 2:44 PM, Pavel Stehule  wrote:

> I think the way forward is to remove the restriction such that data
> returning queries must be PERFORM'd
> 
> I disagree, current rule has sense.

Perhaps a DECLARE FUNCTION attribute that turns off the functionality, then?

Best,

David



-- 
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] PL/pgSQL PERFORM with CTE

[Merlin Moncure]

On Tue, Aug 20, 2013 at 7:44 AM, Pavel Stehule  wrote:
>
>
>
> 2013/8/20 Merlin Moncure 
>>
>> On Tue, Aug 20, 2013 at 7:25 AM, Andres Freund 
>> wrote:
>> > On 2013-08-20 14:15:55 +0200, David E. Wheeler wrote:
>> >> Hi Pavel,
>> >>
>> >> On Aug 20, 2013, at 2:11 PM, Pavel Stehule 
>> >> wrote:
>> >>
>> >> >> david=# DO $$
>> >> >> david$# BEGIN
>> >> >> david$# WITH now AS (SELECT now())
>> >> >> david$# PERFORM * from now;
>> >> >> david$# END;
>> >> >> david$# $$;
>> >> >> ERROR:  syntax error at or near "PERFORM"
>> >> >> LINE 4: PERFORM * from now;
>> >> >> ^
>> >> >> Parser bug in PL/pgSQL, perhaps?
>> >> >
>> >> > no
>> >> >
>> >> > you cannot use a PL/pgSQL statement inside SQL statement.
>> >>
>> >> Well, there ought to be *some* way to tell PL/pgSQL to discard the
>> >> result. Right now I am adding a variable to select into but never 
>> >> otherwise
>> >> use. Inelegant, IMHO. Perhaps I’m missing some other way to do it?
>> >>
>> >> If so, it would help if the hint suggesting the use of PERFORM pointed
>> >> to such alternatives.
>> >
>> > Not that that's elegant but IIRC PERFORM (WITH ...) ought to work. I
>> > don't think the intermingled plpgsql/sql grammars allow a nice way right
>> > now.
>>
>> I think the way forward is to remove the restriction such that data
>> returning queries must be PERFORM'd
>
>
> I disagree, current rule has sense.

Curious what your thinking is there.

merlin


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

Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])

[Stephen Frost]

* Amit Kapila (amit.kapil...@gmail.com) wrote:
> Okay, let us decide how things will be if we disable by default:
>1. initdb won't create any empty auto file, rather the file will be
> created first time user uses ALTER SYSTEM.

I'm not particularly set on this..  Why not create the file initially?

>2. Alter system should issue warning, if it detects that feature is
> disabled (for this we need to error detection code
>during parsing of postgresql.conf as it was previously)

I agree that it should complain through a warning or perhaps an error
(to be honest, I like error more, but there may be good reasons against
that).

>3. postgresql.conf will contain include directive in below form:
>#include = 'postgresql.auto.conf'
>Whenever user wants to use Alter System, he needs to enable it
> after first time using ALTER SYSTEM.

That I don't like..  You should be able to enable it and have things
"work" without having to run ALTER SYSTEM first..

> One question here, if somebody just enables it without using ALTER
> SYSTEM, should it throw any error on not finding auto file or just
> ignore it?

I'd prefer that it error if it can't find an included file.

> > What about include directives?
> 
> Sorry, I didn't get which parameters you are referring by include directives?

Literally, the above 'include' in postgresql.conf.  Would that only be
dealt with as a parse-time thing, or should it be more like a GUC which
could then be set through ALTER SYSTEM?

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] PL/pgSQL PERFORM with CTE

[Pavel Stehule]

2013/8/20 David E. Wheeler 

> On Aug 20, 2013, at 2:41 PM, Pavel Stehule 
> wrote:
>
> > yes, in this context you should not use a PERFORM
> >
> > PL/pgSQL protect you before useless queries - so you can use a CTE
> without returned result directly or CTE with result via PERFORM statement
> (and in this case it must be unmodifing CTE).
> >
> > Sorry, I don't see any problem - why you return some from CTE and then
> you throw this result?
>
> I am passing the values returned from a CTE to a call to pg_notify(). I do
> not care to collect the output of pg_notify(), which returns VOID.
>

it is little bit different issue - PL/pgSQL doesn't check if returned type
is VOID - it can be allowed, I am thinking. So check of empty result can be
enhanced.

Regards

Pavel


>
> Best,
>
> David
>
>

Re: [HACKERS] PL/pgSQL PERFORM with CTE

[Marko Tiikkaja]

On 8/20/13 2:53 PM, Pavel Stehule wrote:

2013/8/20 David E. Wheeler 

I am passing the values returned from a CTE to a call to pg_notify(). I do
not care to collect the output of pg_notify(), which returns VOID.



it is little bit different issue - PL/pgSQL doesn't check if returned type
is VOID - it can be allowed, I am thinking. So check of empty result can be
enhanced.


That still doesn't help at all in the case where the function returns 
something, but you simply don't care about the result.


That said, I don't think this issue is big enough to start radically 
changing how SELECT without INTO works -- you can always get around this 
limitation by SELECTing into a variable, as David mentioned in his 
original message.  It's annoying, but it works.



Regards,
Marko Tiikkaja



--
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] PL/pgSQL PERFORM with CTE

[David E. Wheeler]

On Aug 20, 2013, at 2:53 PM, Pavel Stehule  wrote:

>> I am passing the values returned from a CTE to a call to pg_notify(). I do 
>> not care to collect the output of pg_notify(), which returns VOID.
> 
> it is little bit different issue - PL/pgSQL doesn't check if returned type is 
> VOID - it can be allowed, I am thinking. So check of empty result can be 
> enhanced.

I am confused. I do not need to check the result (except via FOUND). But I am 
sure I can think of other situations where I am calling something where I do 
not care about the result, even if it returns one.

Best,

David



-- 
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] PL/pgSQL PERFORM with CTE

[Pavel Stehule]

2013/8/20 David E. Wheeler 

> On Aug 20, 2013, at 2:53 PM, Pavel Stehule 
> wrote:
>
> >> I am passing the values returned from a CTE to a call to pg_notify(). I
> do not care to collect the output of pg_notify(), which returns VOID.
> >
> > it is little bit different issue - PL/pgSQL doesn't check if returned
> type is VOID - it can be allowed, I am thinking. So check of empty result
> can be enhanced.
>
> I am confused. I do not need to check the result (except via FOUND). But I
> am sure I can think of other situations where I am calling something where
> I do not care about the result, even if it returns one.
>

When you would to ignore result, then you should to use a PERFORM -
actually, it is limited now and should be fixed. Have no problem with it.

I don't would to enable a free unbound statement that returns result.

Regards

Pavel


>
> Best,
>
> David
>
>

Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])

[Stephen Frost]

* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
> Amit Kapila escribió:
> >3. postgresql.conf will contain include directive in below form:
> >#include = 'postgresql.auto.conf'
> >Whenever user wants to use Alter System, he needs to enable it
> > after first time using ALTER SYSTEM.
> 
> This seems wrong to me.  If the auto file is read by an include line in
> postgresql.conf, what is its priority w.r.t. files placed in an
> hypothetical conf.d directory?  

This could be handled by a simple ordering with "last one wins".  I
don't particularly like that though..  My gut feeling is that I'd like
something to complain if there's more than one value set for the same
GUC..

> Hopefully snippets put in conf.d/ by
> puppet/chef will override the settings in postgresql.conf (i.e. conf.d/
> should be processed after postgresql.conf, not before); and hopefully
> ALTER SYSTEM will in turn override conf.d.  I see no way to have ALTER
> SYSTEM handled by an include line, yet still have it override conf.d.

With includedir and include directives, and postgresql.conf setting a
defined ordering, with last-wins, you could simply have the 'includedir'
for conf.d come before the 'include' for auto.conf.

> If we want to make ALTER SYSTEM disable-able from postgresql.conf, I
> think it should be an explicit option, something like
> enable_alter_system = on
> or something like that.

I really don't like this approach- I'd much rather we have a general
include mechanism than special "alter-system" GUCs.

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])

[Amit Kapila]

On Tue, Aug 20, 2013 at 6:21 PM, Stephen Frost  wrote:
> * Amit Kapila (amit.kapil...@gmail.com) wrote:
>> Okay, let us decide how things will be if we disable by default:
>>1. initdb won't create any empty auto file, rather the file will be
>> created first time user uses ALTER SYSTEM.
>
> I'm not particularly set on this..  Why not create the file initially?
   If by default this feature needs to be disabled, then it is not
must to have at initdb time.
   OTOH if we want to enable it by default, then we need to get this
created at initdb time else it will give error during
   system startup (an error can occur during startup when it will
parse postgresql.conf and didn't find the file
   mentioned in include directive).

   Also you mentioned below line upthread which I understood as you
don't like idea of creating empty file at initdb
   time:
   "If it's enabled by default, then we need to ship an 'auto' file which is
empty by default...  I don't particularly like that"


>>2. Alter system should issue warning, if it detects that feature is
>> disabled (for this we need to error detection code
>>during parsing of postgresql.conf as it was previously)
>
> I agree that it should complain through a warning or perhaps an error
> (to be honest, I like error more, but there may be good reasons against
> that).
>
>>3. postgresql.conf will contain include directive in below form:
>>#include = 'postgresql.auto.conf'
>>Whenever user wants to use Alter System, he needs to enable it
>> after first time using ALTER SYSTEM.
>
> That I don't like..  You should be able to enable it and have things
> "work" without having to run ALTER SYSTEM first..

   This was actually linked to point 1 mentioned above, if we create
empty file at initdb time, then there should not be
   any problem.

   One other option to disable as suggested by Alvaro is have another
config parameter to enable/disable Alter
   System, if we can do that way, the solution will be much neater.

>> One question here, if somebody just enables it without using ALTER
>> SYSTEM, should it throw any error on not finding auto file or just
>> ignore it?
>
> I'd prefer that it error if it can't find an included file.
>
>> > What about include directives?
>>
>> Sorry, I didn't get which parameters you are referring by include directives?
>
> Literally, the above 'include' in postgresql.conf.  Would that only be
> dealt with as a parse-time thing, or should it be more like a GUC which
> could then be set through ALTER SYSTEM?

 I think if we choose to use include directive as a way to
enable/disable this feature, it will not be good to allow change of
this parameter with Alter System.


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] PL/pgSQL PERFORM with CTE

[David E. Wheeler]

On Aug 20, 2013, at 3:05 PM, Pavel Stehule  wrote:

> When you would to ignore result, then you should to use a PERFORM - actually, 
> it is limited now and should be fixed. Have no problem with it.

Glad to have you on board. :-)

> I don't would to enable a free unbound statement that returns result. 

I have no pony in that race. I think it is useful, though I prefer to unit test 
things enough that I would be fine without it.

But even without it, there may be times when I want to discard a result in a 
function that *does* return a value -- likely a different value. So there needs 
to be a way to distinguish statements that should return a value and those that 
do not.

Best,

David



-- 
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] PL/pgSQL PERFORM with CTE

[Pavel Stehule]

2013/8/20 David E. Wheeler 

> On Aug 20, 2013, at 3:05 PM, Pavel Stehule 
> wrote:
>
> > When you would to ignore result, then you should to use a PERFORM -
> actually, it is limited now and should be fixed. Have no problem with it.
>
> Glad to have you on board. :-)
>
> > I don't would to enable a free unbound statement that returns result.
>
> I have no pony in that race. I think it is useful, though I prefer to unit
> test things enough that I would be fine without it.
>
> But even without it, there may be times when I want to discard a result in
> a function that *does* return a value -- likely a different value. So there
> needs to be a way to distinguish statements that should return a value and
> those that do not.
>

can you show some examples, please

Pavel


> Best,
>
> David
>
>

Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])

[Stephen Frost]

* Amit Kapila (amit.kapil...@gmail.com) wrote:
> On Tue, Aug 20, 2013 at 6:21 PM, Stephen Frost  wrote:
> > I'm not particularly set on this..  Why not create the file initially?
>If by default this feature needs to be disabled, then it is not
> must to have at initdb time.

I don't see how the two are related.  You could never use two-phase
commit (could even disable it), yet we still create things in the data
directory to support it.  Having a file in the data directory which
isn't used by default isn't that big a deal, imv.

>Also you mentioned below line upthread which I understood as you
> don't like idea of creating empty file at initdb
>time:
>"If it's enabled by default, then we need to ship an 'auto' file which is
> empty by default...  I don't particularly like that"

What I didn't like was having an empty file be accepted as 'valid', but
you need to have some kind of bootstrap mechanism.  Telling users "run
this command and then ignore the warning" is certainly bad.  A better
option might be to have a *non-empty* auto.conf be generated, where all
it has in it is some kind of identifier, perhaps even just
"enable_alter_system = on" which we could then key off of to see if
ALTER SYSTEM has been set up to be allowed or not.

>  I think if we choose to use include directive as a way to
> enable/disable this feature, it will not be good to allow change of
> this parameter with Alter System.

I agree, but then we need to add it to the list of things ALTER SYSTEM
can't modify (if the include is implemented as a GUC, that is; I've not
looked).

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] PL/pgSQL PERFORM with CTE

[David E. Wheeler]

On Aug 20, 2013, at 3:18 PM, Pavel Stehule  wrote:

> can you show some examples, please

This is not dissimilar to what I am actually doing:

CREATE TABLE foo (id SERIAL PRIMARY KEY, name TEXT);

CREATE OR REPLACE FUNCTION shipit (
VARIADIC things TEXT[]
) RETURNS BOOL LANGUAGE plpgsql AS $$
BEGIN
WITH inserted AS (
INSERT INTO foo (name)
SELECT * FROM unnest(things)
RETURNING id
)
PERFORM pg_notify(
'inserted ids',
ARRAY(SELECT * FROM inserted)::text
);
RETURN FOUND;
END;
$$;

Only I am using a dummy row variable instead of PERFORM, of course.

Best,

David



-- 
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] GSOC13 proposal - extend RETURNING syntax

[Boszormenyi Zoltan]

Hi,

2013-08-19 21:52 keltezéssel, Boszormenyi Zoltan írta:

2013-08-19 21:21 keltezéssel, Karol Trzcionka írta:

W dniu 19.08.2013 19:56, Boszormenyi Zoltan pisze:

* Does it apply cleanly to the current git master?

No. There's a reject in src/backend/optimizer/plan/initsplan.c

Thank you, merged in attached version.

* Does it include reasonable tests?

Yes but the test fails after trying to fix the rejected chunk of the
patch.

It fails because the "HINT" was changed, fixed.
That version merges some nested "ifs" left over from earlier work.


I tried to compile your v5 patch and I got:

initsplan.c: In function ‘add_vars_to_targetlist’:
initsplan.c:216:26: warning: ‘rel’ may be used uninitialized in this function 
[-Wmaybe-uninitialized]

rel->reltargetlist = lappend(rel->reltargetlist,
^

You shouldn't change the assignment at declaration:

- RelOptInfo *rel = find_base_rel(root, var->varno);
+ RelOptInfo *rel;
...
+ if (root->parse->commandType == CMD_UPDATE)
+ {
... (code using rel)
+ }
+ rel = find_base_rel(root, varno);


Let me say it again: the new code in initsplan.c::add_vars_to_targetlist() is 
fishy.
The compiler says that "rel" used on line 216 may be uninitialized.

Keeping it that way passes "make check", perhaps "rel" was initialized
in a previous iteration of "foreach(temp, vars)", possibly in the
else if (IsA(node, PlaceHolderVar))
branch, which means that "PlaceHolderInfo *phinfo" may be interpreted
as RelOptInfo *, stomping on memory.

Moving the assignment back to the declaration makes "make check"
fail with the attached regression.diffs file.

Initializing it as "RelOptInfo *rel = NULL;" makes the regression check
die with a segfault, obviously.

Change the code to avoid the warning and still produce the wanted effect.

Best regards,
Zoltán Böszörményi

--
--
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt, Austria
Web: http://www.postgresql-support.de
 http://www.postgresql.at/

*** 
/home/zozo/crosscolumn/review/returning-before-after/postgresql/src/test/regress/expected/returning_before_after.out
2013-08-20 15:01:03.365314482 +0200
--- 
/home/zozo/crosscolumn/review/returning-before-after/postgresql/src/test/regress/results/returning_before_after.out
 2013-08-20 15:30:21.091641454 +0200
***
*** 6,47 
);
  INSERT INTO foo VALUES (1, 'x'),(2,'y');
  UPDATE foo SET bar1=bar1+1 RETURNING before.*, bar1, bar2;
!  bar1 | bar2 | bar1 | bar2 
! --+--+--+--
! 1 | x|2 | x
! 2 | y|3 | y
! (2 rows)
! 
  UPDATE foo SET bar1=bar1-1 RETURNING after.bar1, before.bar1*2;
!  bar1 | ?column? 
! --+--
! 1 |4
! 2 |6
! (2 rows)
! 
  UPDATE foo SET bar1=bar1+1, bar2=bar2 || 'z' RETURNING before.*, after.*;
!  bar1 | bar2 | bar1 | bar2 
! --+--+--+--
! 1 | x|2 | xz
! 2 | y|3 | yz
! (2 rows)
! 
  -- check single after
  UPDATE foo SET bar1=bar1+1, bar2=bar2 || 'a' RETURNING after.*;
!  bar1 | bar2 
! --+--
! 3 | xza
! 4 | yza
! (2 rows)
! 
  -- check single before
  UPDATE foo SET bar1=bar1+1, bar2=bar2 || 'b' RETURNING before.*;
!  bar1 | bar2 
! --+--
! 3 | xza
! 4 | yza
! (2 rows)
! 
  -- it should fail
  UPDATE foo SET bar1=bar1+before.bar1 RETURNING before.*;
  ERROR:  missing FROM-clause entry for table "before"
--- 6,22 
);
  INSERT INTO foo VALUES (1, 'x'),(2,'y');
  UPDATE foo SET bar1=bar1+1 RETURNING before.*, bar1, bar2;
! ERROR:  no relation entry for relid 2
  UPDATE foo SET bar1=bar1-1 RETURNING after.bar1, before.bar1*2;
! ERROR:  no relation entry for relid 3
  UPDATE foo SET bar1=bar1+1, bar2=bar2 || 'z' RETURNING before.*, after.*;
! ERROR:  no relation entry for relid 2
  -- check single after
  UPDATE foo SET bar1=bar1+1, bar2=bar2 || 'a' RETURNING after.*;
! ERROR:  no relation entry for relid 3
  -- check single before
  UPDATE foo SET bar1=bar1+1, bar2=bar2 || 'b' RETURNING before.*;
! ERROR:  no relation entry for relid 2
  -- it should fail
  UPDATE foo SET bar1=bar1+before.bar1 RETURNING before.*;
  ERROR:  missing FROM-clause entry for table "before"
***
*** 53,90 
   ^
  -- test before/after aliases
  UPDATE foo AS before SET bar1=bar1+1 RETURNING before.*,after.*;
!  bar1 | bar2 | bar1 | bar2 
! --+--+--+--
! 5 | xzab |5 | xzab
! 6 | yzab |6 | yzab
! (2 rows)
! 
  UPDATE foo AS after SET bar1=bar1-1 RETURNING before.*,after.*;
!  bar1 | bar2 | bar1 | bar2 
! --+--+--+--
! 5 | xzab |4 | xzab
! 6 | yzab |5 | yzab
! (2 rows)
! 
  -- test inheritance
  CREATE TABLE foo2 (bar INTEGER) INHERITS(foo);
  INSERT INTO foo2 VALUES (1,'b',5);
  UPDATE foo2 SET bar1=bar1*2, bar=bar1+5, bar2=bar1::text || bar::text 
RETURNING before.*, after.*, *;
!  bar1 | bar2 | bar | bar1 | bar2 | bar | 

Re: [HACKERS] PL/pgSQL PERFORM with CTE

[Pavel Stehule]

2013/8/20 David E. Wheeler 

> On Aug 20, 2013, at 3:18 PM, Pavel Stehule 
> wrote:
>
> > can you show some examples, please
>
> This is not dissimilar to what I am actually doing:
>
> CREATE TABLE foo (id SERIAL PRIMARY KEY, name TEXT);
>
> CREATE OR REPLACE FUNCTION shipit (
> VARIADIC things TEXT[]
> ) RETURNS BOOL LANGUAGE plpgsql AS $$
> BEGIN
> WITH inserted AS (
> INSERT INTO foo (name)
> SELECT * FROM unnest(things)
> RETURNING id
> )
> PERFORM pg_notify(
> 'inserted ids',
> ARRAY(SELECT * FROM inserted)::text
> );
> RETURN FOUND;
> END;
> $$;
>
> Only I am using a dummy row variable instead of PERFORM, of course.
>

pg_notify returns void, so there are no necessary casting to void

so enhanced check - so all returned columns are void should be enough

Regards

Pavel


>
> Best,
>
> David
>
>

Re: [HACKERS] Fix Windows socket error checking for MinGW

[Andrew Dunstan]

On 08/19/2013 11:36 PM, Michael Cronenworth wrote:

On 08/19/2013 07:35 PM, Noah Misch wrote:

That was option #1.  (You weren't planning to change just the one symbol
causing the failure at hand, were you?)  Reasonable choice.  The 
point in the
code comment quoted above looks bad, but the lack of reports of that 
nature
against official 9.2 binaries corroborates it having not been a 
problem yet.
The only non-socket use I see for the constants in question is the 
EINTR test

in XLogWrite(), which probably can't happen on Windows.


Redefining compiler constants is bad bandaid. A similar bandaid was in 
place to begin with caused this problem. If you believe PostgreSQL 
will never use code that needs the true errno value then go ahead with 
Option 1.



No the reverse is the case. The real problem is that the bandaid was not 
applied sufficiently widely. What we propose is exactly what is already 
in use for the Microsoft compilers, and has thus been well and truly 
tested over some years.


Changing these definitions doesn't change the value of errno in the 
slightest - it only changes the values that we test for.


The caveat in the MSVC-specific code that Noah pointed to still applies, 
but it appears that we don't in fact use these constants anywhere other 
than in relation to sockets.


I'm about to update my buildfarm member jacana so it has the latest 
mingw-w64 compiler and this should exhibit this error. Then I'll apply a 
patch along the lines of option 1. If nothing breaks, I'll backpatch 
that to 9.1 where we enabled use of this compiler.


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] PL/pgSQL PERFORM with CTE

[David E. Wheeler]

On Aug 20, 2013, at 3:38 PM, Pavel Stehule  wrote:

> pg_notify returns void, so there are no necessary casting to void
> 
> so enhanced check - so all returned columns are void should be enough

What if I call another function I wrote myself that returns an INT, but I do 
not care about the INT? Maybe that function does the insert and returns the 
number of inserted rows.

I can think of all kinds of reasons this might be the case; whether they are 
good or bad approaches is immaterial: sometimes you work with what you have.

I am find with PERFORM to determine when a query's results should be discarded. 
I just think it needs to cover a few more cases.

Best,

David

-- 
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] PL/pgSQL PERFORM with CTE

[Pavel Stehule]

2013/8/20 David E. Wheeler 

> On Aug 20, 2013, at 3:38 PM, Pavel Stehule 
> wrote:
>
> > pg_notify returns void, so there are no necessary casting to void
> >
> > so enhanced check - so all returned columns are void should be enough
>
> What if I call another function I wrote myself that returns an INT, but I
> do not care about the INT? Maybe that function does the insert and returns
> the number of inserted rows.
>
> I can think of all kinds of reasons this might be the case; whether they
> are good or bad approaches is immaterial: sometimes you work with what you
> have.
>
> I am find with PERFORM to determine when a query's results should be
> discarded. I just think it needs to cover a few more cases.
>

yes

I understand. I'll look, how PERFORM can be fixed

Regards

Pavel


>
> Best,
>
> David

Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])

[Amit Kapila]

On Tue, Aug 20, 2013 at 6:55 PM, Stephen Frost  wrote:
> * Amit Kapila (amit.kapil...@gmail.com) wrote:
>> On Tue, Aug 20, 2013 at 6:21 PM, Stephen Frost  wrote:
>> > I'm not particularly set on this..  Why not create the file initially?
>>If by default this feature needs to be disabled, then it is not
>> must to have at initdb time.
>
> I don't see how the two are related.  You could never use two-phase
> commit (could even disable it), yet we still create things in the data
> directory to support it.  Having a file in the data directory which
> isn't used by default isn't that big a deal, imv.

   Yes, it can be done, what I wanted to say it is not a "must",
rather a good to have thing.

>>Also you mentioned below line upthread which I understood as you
>> don't like idea of creating empty file at initdb
>>time:
>>"If it's enabled by default, then we need to ship an 'auto' file which is
>> empty by default...  I don't particularly like that"
>
> What I didn't like was having an empty file be accepted as 'valid', but
> you need to have some kind of bootstrap mechanism.  Telling users "run
> this command and then ignore the warning" is certainly bad.  A better
> option might be to have a *non-empty* auto.conf be generated, where all
> it has in it is some kind of identifier, perhaps even just
> "enable_alter_system = on" which we could then key off of to see if
> ALTER SYSTEM has been set up to be allowed or not.

Wouldn't it be complicated to handle this way rather then by having
include directive.
If include directive is enabled then the autofile will be read else no
need to read it.

If we want to have separate identifier to judge ALTER SYSTEM is enable
or not, then it is better to go with a new GUC.

>>  I think if we choose to use include directive as a way to
>> enable/disable this feature, it will not be good to allow change of
>> this parameter with Alter System.
>
> I agree, but then we need to add it to the list of things ALTER SYSTEM
> can't modify (if the include is implemented as a GUC, that is; I've not
> looked).

   I have checked and it seems to be just parse time thing.

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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])

[Stephen Frost]

* Amit Kapila (amit.kapil...@gmail.com) wrote:
> Wouldn't it be complicated to handle this way rather then by having
> include directive.

You misunderstood.  I was suggesting a setup along these lines:

postgresql.conf:
  # include = 'auto.conf'  # Disabled by default

auto.conf:
  
  # MANAGED BY ALTER SYSTEM -- DO NOT MODIFY BY HAND
  

  # auto.conf not processed unless included by postgresql.conf
  # If included by postgresql.conf, then this will turn on the
  # ALTER SYSTEM command.
  enable_alter_system = on 

Which would give us the ability to independently turn on/off ALTER
SYSTEM from including auto.conf.

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])

[Amit Kapila]

On Tue, Aug 20, 2013 at 7:51 PM, Stephen Frost  wrote:
> * Amit Kapila (amit.kapil...@gmail.com) wrote:
>> Wouldn't it be complicated to handle this way rather then by having
>> include directive.
>
> You misunderstood.  I was suggesting a setup along these lines:
>
> postgresql.conf:
>   # include = 'auto.conf'  # Disabled by default
>
> auto.conf:
>   
>   # MANAGED BY ALTER SYSTEM -- DO NOT MODIFY BY HAND
>   
>
>   # auto.conf not processed unless included by postgresql.conf
>   # If included by postgresql.conf, then this will turn on the
>   # ALTER SYSTEM command.
>   enable_alter_system = on
>
> Which would give us the ability to independently turn on/off ALTER
> SYSTEM from including auto.conf.

So let me try to explain what I understood from above:

1. enable_alter_system a new GUC whose default value =off.
2. Alter System will check this variable and return error (not
allowed), if this parameter is off.
3. Now if user enables include directive in postgresql.conf, it will
enable Alter System as value of
enable_alter_system is on.
4. User can run Alter System command to disable Alter System
"enable_alter_system = off".
Now even though include directive is enabled, but new Alter System
commands will not work, however
existing parameter's take into effect on restart/sighup.


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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])

[Stephen Frost]

* Amit Kapila (amit.kapil...@gmail.com) wrote:
> So let me try to explain what I understood from above:
> 
> 1. enable_alter_system a new GUC whose default value =off.
> 2. Alter System will check this variable and return error (not
> allowed), if this parameter is off.
> 3. Now if user enables include directive in postgresql.conf, it will
> enable Alter System as value of
> enable_alter_system is on.
> 4. User can run Alter System command to disable Alter System
> "enable_alter_system = off".
> Now even though include directive is enabled, but new Alter System
> commands will not work, however
> existing parameter's take into effect on restart/sighup.

Yes.  Not sure that it'd be terribly likely for a user to do that, but
if they do it, so be it.

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

[Karol Trzcionka]

Thank you for the review and tests. New version introduce a lot of
improvements:
- Fix regression test for view (wrong table_name)
- Add regression test for inheritance
- Delete hack in initsplan.c (now we ignore all RTE_BEFORE) - the
uninitialized issue
- Revert changing varno in add_vars_to_targetlist
- Add all "before" variables to targetlist
- Avoid adding variables to slot for AFTER.
- Treat varnoold like a flag - prevent from adjustment if RTE_BEFORE
- All before/after are now set on OUTER_VAR
- Rename fix_varno_varattno to bind_returning_variables
- Add comment about bind_returning_variables
- Remove unneeded code in fix_join_expr_mutator (it was changing varno
of RTE_BEFORE - now there is not any var with varno assigned to it)
Regards,
Karol Trzcionka
diff --git a/doc/src/sgml/ref/update.sgml b/doc/src/sgml/ref/update.sgml
index 90b9208..eba35f0 100644
--- a/doc/src/sgml/ref/update.sgml
+++ b/doc/src/sgml/ref/update.sgml
@@ -194,12 +194,27 @@ UPDATE [ ONLY ] table_name [ * ] [
 output_expression
 
  
-  An expression to be computed and returned by the UPDATE
-  command after each row is updated.  The expression can use any
-  column names of the table named by table_name
-  or table(s) listed in FROM.
-  Write * to return all columns.
+  An expression to be computed and returned by the
+  UPDATE command either before or after (prefixed with
+  BEFORE. and AFTER.,
+  respectively) each row is updated.  The expression can use any
+  column names of the table named by table_name or table(s) listed in
+  FROM.  Write AFTER.* to return all 
+  columns after the update. Write BEFORE.* for all
+  columns before the update. Write * to return all
+  columns after update and all triggers fired (these values are in table
+  after command). You may combine BEFORE, AFTER and raw columns in the
+  expression.
  
+ 
+ Mixing table names or aliases named before or after with the
+ above will result in confusion and suffering.  If you happen to
+ have a table called before or
+ after, alias it to something else when using
+ RETURNING.
+ 
+
 

 
@@ -287,15 +302,16 @@ UPDATE weather SET temp_lo = temp_lo+1, temp_hi = temp_lo+15, prcp = DEFAULT
   
 
   
-   Perform the same operation and return the updated entries:
+   Perform the same operation and return information on the changed entries:
 
 
 UPDATE weather SET temp_lo = temp_lo+1, temp_hi = temp_lo+15, prcp = DEFAULT
   WHERE city = 'San Francisco' AND date = '2003-07-03'
-  RETURNING temp_lo, temp_hi, prcp;
+  RETURNING temp_lo AS new_low, temp_hi AS new_high, BEFORE.temp_hi/BEFORE.temp_low AS old_ratio, AFTER.temp_hi/AFTER.temp_low AS new_ratio prcp;
 
   
 
+
   
Use the alternative column-list syntax to do the same update:
 
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index d86e9ad..fafd311 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -2335,7 +2335,7 @@ ExecASUpdateTriggers(EState *estate, ResultRelInfo *relinfo)
 TupleTableSlot *
 ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
 	 ResultRelInfo *relinfo,
-	 ItemPointer tupleid, TupleTableSlot *slot)
+	 ItemPointer tupleid, TupleTableSlot *slot, TupleTableSlot **planSlot)
 {
 	TriggerDesc *trigdesc = relinfo->ri_TrigDesc;
 	HeapTuple	slottuple = ExecMaterializeSlot(slot);
@@ -2381,6 +2381,7 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
 	if (newSlot != NULL)
 	{
 		slot = ExecFilterJunk(relinfo->ri_junkFilter, newSlot);
+		*planSlot = newSlot;
 		slottuple = ExecMaterializeSlot(slot);
 		newtuple = slottuple;
 	}
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 15f5dcc..06ebaf3 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -603,7 +603,7 @@ ExecUpdate(ItemPointer tupleid,
 		resultRelInfo->ri_TrigDesc->trig_update_before_row)
 	{
 		slot = ExecBRUpdateTriggers(estate, epqstate, resultRelInfo,
-	tupleid, slot);
+	tupleid, slot, &planSlot);
 
 		if (slot == NULL)		/* "do nothing" */
 			return NULL;
@@ -737,6 +737,7 @@ lreplace:;
 		   hufd.xmax);
 	if (!TupIsNull(epqslot))
 	{
+		planSlot = epqslot;
 		*tupleid = hufd.ctid;
 		slot = ExecFilterJunk(resultRelInfo->ri_junkFilter, epqslot);
 		tuple = ExecMaterializeSlot(slot);
diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c
index 908f397..461ec4f 100644
--- a/src/backend/nodes/nodeFuncs.c
+++ b/src/backend/nodes/nodeFuncs.c
@@ -1987,6 +1987,7 @@ range_table_walker(List *rtable,
 		{
 			case RTE_RELATION:
 			case RTE_CTE:
+			case RTE_BEFORE:
 /* nothing to do */
 break;
 			case RTE_SUBQUERY:
@@ -2701,6 +2702,7 @@ range_table_mutator(List *rtable,
 		{
 			case RTE_RELATION:
 			case RTE_CTE:
+			case RTE_BEFORE:
 /* we don't bother to copy e

Re: [HACKERS] Parsing bool type value

[Amit Kapila]

On Tue, Aug 20, 2013 at 1:11 PM, Sawada Masahiko  wrote:
> Hi all,
>
> Taking a look at PostgreSQL HEAD today, I noticed that currently
> PostgreSQL allows "of" value as bool type value.
> So user can execute the following SQL.
>
> =# SET enbale_seqscan TO of;
>
> And I read the source code related to parsing bool value.
> It compare TWO characters "off" and the setting value in
> parse_bool_with_len() function.
> Should we deny the "of" value as bool type value?

 When I checked the manual for values of bool types, it says as follows:
  " Boolean values can be written as on, off, true, false, yes, no, 1,
0 (all case-insensitive) or any unambiguous prefix of these."
   Now "of" can be considered as unambiguous prefix of "off", so it
might be intentional.
   Please refer below link for more detailed description:
   
http://www.postgresql.org/docs/devel/static/config-setting.html#CONFIG-SETTING-NAMES-VALUES



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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])

[Alvaro Herrera]

Stephen Frost escribió:
> * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:

> > Hopefully snippets put in conf.d/ by
> > puppet/chef will override the settings in postgresql.conf (i.e. conf.d/
> > should be processed after postgresql.conf, not before); and hopefully
> > ALTER SYSTEM will in turn override conf.d.  I see no way to have ALTER
> > SYSTEM handled by an include line, yet still have it override conf.d.
> 
> With includedir and include directives, and postgresql.conf setting a
> defined ordering, with last-wins, you could simply have the 'includedir'
> for conf.d come before the 'include' for auto.conf.

Uh, I was thinking that conf.d would be read by the system
automatically, not because of an includedir line in postgresql.conf.

-- 
Álvaro Herrerahttp://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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])

[Stephen Frost]

* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
> Stephen Frost escribió:
> > With includedir and include directives, and postgresql.conf setting a
> > defined ordering, with last-wins, you could simply have the 'includedir'
> > for conf.d come before the 'include' for auto.conf.
> 
> Uh, I was thinking that conf.d would be read by the system
> automatically, not because of an includedir line in postgresql.conf.

I'd much rather have an includedir directive than some hard-coded or
command-line option to read the directory..  The directory should live
in /etc/postgresql/X.Y/cluster/ on at least Debian derivatives..

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])

[Alvaro Herrera]

Stephen Frost escribió:
> * Amit Kapila (amit.kapil...@gmail.com) wrote:
> > So let me try to explain what I understood from above:
> > 
> > 1. enable_alter_system a new GUC whose default value =off.
> > 2. Alter System will check this variable and return error (not
> > allowed), if this parameter is off.
> > 3. Now if user enables include directive in postgresql.conf, it will
> > enable Alter System as value of
> > enable_alter_system is on.
> > 4. User can run Alter System command to disable Alter System
> > "enable_alter_system = off".
> > Now even though include directive is enabled, but new Alter System
> > commands will not work, however
> > existing parameter's take into effect on restart/sighup.
> 
> Yes.  Not sure that it'd be terribly likely for a user to do that, but
> if they do it, so be it.

With this design, if you put enable_alter_system=off in auto.conf, there
is no way for the user to enable alter system again short of editing a
file in the data directory.  I think this is one of the things that was
"forbidden" by policy; only files in the config directory needs to be
edited.

What I was proposing upthread is that enable_alter_system=off/on would
be present in postgresql.conf, and there is no include line for
auto.conf.  That way, if the user wishes to enable/disable the feature,
they need to edit postgresql.conf to do so.  ALTER SYSTEM doesn't offer
a way to disable itself.  If ALTER SYSTEM is disabled via
enable_alter_system=off in postgresql.conf, the settings in auto.conf
are not read.

-- 
Álvaro Herrerahttp://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] Parsing bool type value

[Sawada Masahiko]

On Tue, Aug 20, 2013 at 11:53 PM, Amit Kapila  wrote:
> On Tue, Aug 20, 2013 at 1:11 PM, Sawada Masahiko  
> wrote:
>> Hi all,
>>
>> Taking a look at PostgreSQL HEAD today, I noticed that currently
>> PostgreSQL allows "of" value as bool type value.
>> So user can execute the following SQL.
>>
>> =# SET enbale_seqscan TO of;
>>
>> And I read the source code related to parsing bool value.
>> It compare TWO characters "off" and the setting value in
>> parse_bool_with_len() function.
>> Should we deny the "of" value as bool type value?
>
>  When I checked the manual for values of bool types, it says as follows:
>   " Boolean values can be written as on, off, true, false, yes, no, 1,
> 0 (all case-insensitive) or any unambiguous prefix of these."
>Now "of" can be considered as unambiguous prefix of "off", so it
> might be intentional.
>Please refer below link for more detailed description:
>
> http://www.postgresql.org/docs/devel/static/config-setting.html#CONFIG-SETTING-NAMES-VALUES
Thank you for replay.

I have confirmed manual and understood.
And I have understood the comment which is written at
parse_bool_with_len() function.

Thanks!

Regards,

---
Sawada Masahiko


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

Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])

[Alvaro Herrera]

Stephen Frost escribió:
> * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
> > Stephen Frost escribió:
> > > With includedir and include directives, and postgresql.conf setting a
> > > defined ordering, with last-wins, you could simply have the 'includedir'
> > > for conf.d come before the 'include' for auto.conf.
> > 
> > Uh, I was thinking that conf.d would be read by the system
> > automatically, not because of an includedir line in postgresql.conf.
> 
> I'd much rather have an includedir directive than some hard-coded or
> command-line option to read the directory..  The directory should live
> in /etc/postgresql/X.Y/cluster/ on at least Debian derivatives..

The conf.d/ path would be relative to postgresql.conf, so there's no
need for Debian to patch anything.

-- 
Álvaro Herrerahttp://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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])

[Tom Lane]

Alvaro Herrera  writes:
> What I was proposing upthread is that enable_alter_system=off/on would
> be present in postgresql.conf, and there is no include line for
> auto.conf.  That way, if the user wishes to enable/disable the feature,
> they need to edit postgresql.conf to do so.  ALTER SYSTEM doesn't offer
> a way to disable itself.  If ALTER SYSTEM is disabled via
> enable_alter_system=off in postgresql.conf, the settings in auto.conf
> are not read.

Personally, I'd get rid of any explicit "enable_alter_system" variable,
and instead have an include directive for auto.conf.  The functionality
you describe above is then available by commenting or uncommenting the
directive, plus the user has the option to decide where to put the
directive (and thus control which settings can or can't be overridden).
Anything else seems like it's going to be less flexible or else a lot
more complicated.

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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])

[Stephen Frost]

* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
> With this design, if you put enable_alter_system=off in auto.conf, there
> is no way for the user to enable alter system again short of editing a
> file in the data directory.  I think this is one of the things that was
> "forbidden" by policy; only files in the config directory needs to be
> edited.

If you edit it by hand to begin with (setting that parameter to 'off')
then it's reasonable that you may have to edit it by hand again to fix
it.  If we don't want people to "lock themselves out" by using ALTER
SYSTEM to turn it off, then we just disallow that.

> What I was proposing upthread is that enable_alter_system=off/on would
> be present in postgresql.conf, and there is no include line for
> auto.conf.  

I really think that's a terrible approach, to be honest.  I want to see
an 'include' line in postgresql.conf for auto.conf, so the hapless
sysadmin who is trying to figure out what the crazy DBA did has some
clue what to look for.  "enable_alter_system" doesn't tell him diddly
about an 'auto.conf' file which is included in the system config.

> That way, if the user wishes to enable/disable the feature,
> they need to edit postgresql.conf to do so.  ALTER SYSTEM doesn't offer
> a way to disable itself.  

We can simply disallow ALTER SYSTEM from modifying enable_alter_system;
that strikes me as a reasonable thing to do anyway.  What I find a bit
more worrying is what happens if they decide to put
enable_alter_system=off into the postgresql.conf but keep the 'include'
line for auto.conf..  Which goes right back to the question that I had
before around if we want to complain when the same GUC is seen multiple
times during parsing.  It seems like there's no hope for it, given the
way this has been designed, because you *must* set certain parameters
and so you can't simply have them commented out, but those are likely to
be parameters which DBAs will want to change through ALTER SYSTEM.

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])

[Stephen Frost]

* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
> Stephen Frost escribió:
> > * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
> > > Stephen Frost escribió:
> > > > With includedir and include directives, and postgresql.conf setting a
> > > > defined ordering, with last-wins, you could simply have the 'includedir'
> > > > for conf.d come before the 'include' for auto.conf.
> > > 
> > > Uh, I was thinking that conf.d would be read by the system
> > > automatically, not because of an includedir line in postgresql.conf.
> > 
> > I'd much rather have an includedir directive than some hard-coded or
> > command-line option to read the directory..  The directory should live
> > in /etc/postgresql/X.Y/cluster/ on at least Debian derivatives..
> 
> The conf.d/ path would be relative to postgresql.conf, so there's no
> need for Debian to patch anything.

Uhhh, I really don't see that working, at all...

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])

[Tom Lane]

Stephen Frost  writes:
> * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
>> The conf.d/ path would be relative to postgresql.conf, so there's no
>> need for Debian to patch anything.

> Uhhh, I really don't see that working, at all...

Why not?  conf.d is for installable files, AIUI.  What we need to
be writable is auto.conf, but I thought we'd agreed that would live
inside $PGDATA.

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] GSOC13 proposal - extend RETURNING syntax

[Karol Trzcionka]

W dniu 20.08.2013 16:47, Karol Trzcionka pisze:
> Thank you for the review and tests. New version introduce a lot of
> improvements:
> - Fix regression test for view (wrong table_name)
> - Add regression test for inheritance
> - Delete hack in initsplan.c (now we ignore all RTE_BEFORE) - the
> uninitialized issue
> - Revert changing varno in add_vars_to_targetlist
> - Add all "before" variables to targetlist
> - Avoid adding variables to slot for AFTER.
> - Treat varnoold like a flag - prevent from adjustment if RTE_BEFORE
> - All before/after are now set on OUTER_VAR
> - Rename fix_varno_varattno to bind_returning_variables
> - Add comment about bind_returning_variables
> - Remove unneeded code in fix_join_expr_mutator (it was changing varno
> of RTE_BEFORE - now there is not any var with varno assigned to it)
I've just realized the prepare_returning_before() is unneeded right now
so I've removed it. Version 7, hopefully the last. ;)
Regards,
Karol Trzcionka
diff --git a/doc/src/sgml/ref/update.sgml b/doc/src/sgml/ref/update.sgml
index 90b9208..eba35f0 100644
--- a/doc/src/sgml/ref/update.sgml
+++ b/doc/src/sgml/ref/update.sgml
@@ -194,12 +194,27 @@ UPDATE [ ONLY ] table_name [ * ] [
 output_expression
 
  
-  An expression to be computed and returned by the UPDATE
-  command after each row is updated.  The expression can use any
-  column names of the table named by table_name
-  or table(s) listed in FROM.
-  Write * to return all columns.
+  An expression to be computed and returned by the
+  UPDATE command either before or after (prefixed with
+  BEFORE. and AFTER.,
+  respectively) each row is updated.  The expression can use any
+  column names of the table named by table_name or table(s) listed in
+  FROM.  Write AFTER.* to return all 
+  columns after the update. Write BEFORE.* for all
+  columns before the update. Write * to return all
+  columns after update and all triggers fired (these values are in table
+  after command). You may combine BEFORE, AFTER and raw columns in the
+  expression.
  
+ 
+ Mixing table names or aliases named before or after with the
+ above will result in confusion and suffering.  If you happen to
+ have a table called before or
+ after, alias it to something else when using
+ RETURNING.
+ 
+
 

 
@@ -287,15 +302,16 @@ UPDATE weather SET temp_lo = temp_lo+1, temp_hi = temp_lo+15, prcp = DEFAULT
   
 
   
-   Perform the same operation and return the updated entries:
+   Perform the same operation and return information on the changed entries:
 
 
 UPDATE weather SET temp_lo = temp_lo+1, temp_hi = temp_lo+15, prcp = DEFAULT
   WHERE city = 'San Francisco' AND date = '2003-07-03'
-  RETURNING temp_lo, temp_hi, prcp;
+  RETURNING temp_lo AS new_low, temp_hi AS new_high, BEFORE.temp_hi/BEFORE.temp_low AS old_ratio, AFTER.temp_hi/AFTER.temp_low AS new_ratio prcp;
 
   
 
+
   
Use the alternative column-list syntax to do the same update:
 
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index d86e9ad..fafd311 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -2335,7 +2335,7 @@ ExecASUpdateTriggers(EState *estate, ResultRelInfo *relinfo)
 TupleTableSlot *
 ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
 	 ResultRelInfo *relinfo,
-	 ItemPointer tupleid, TupleTableSlot *slot)
+	 ItemPointer tupleid, TupleTableSlot *slot, TupleTableSlot **planSlot)
 {
 	TriggerDesc *trigdesc = relinfo->ri_TrigDesc;
 	HeapTuple	slottuple = ExecMaterializeSlot(slot);
@@ -2381,6 +2381,7 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
 	if (newSlot != NULL)
 	{
 		slot = ExecFilterJunk(relinfo->ri_junkFilter, newSlot);
+		*planSlot = newSlot;
 		slottuple = ExecMaterializeSlot(slot);
 		newtuple = slottuple;
 	}
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 15f5dcc..06ebaf3 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -603,7 +603,7 @@ ExecUpdate(ItemPointer tupleid,
 		resultRelInfo->ri_TrigDesc->trig_update_before_row)
 	{
 		slot = ExecBRUpdateTriggers(estate, epqstate, resultRelInfo,
-	tupleid, slot);
+	tupleid, slot, &planSlot);
 
 		if (slot == NULL)		/* "do nothing" */
 			return NULL;
@@ -737,6 +737,7 @@ lreplace:;
 		   hufd.xmax);
 	if (!TupIsNull(epqslot))
 	{
+		planSlot = epqslot;
 		*tupleid = hufd.ctid;
 		slot = ExecFilterJunk(resultRelInfo->ri_junkFilter, epqslot);
 		tuple = ExecMaterializeSlot(slot);
diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c
index 908f397..461ec4f 100644
--- a/src/backend/nodes/nodeFuncs.c
+++ b/src/backend/nodes/nodeFuncs.c
@@ -1987,6 +1987,7 @@ range_table_walker(List *rtable,
 		{
 			case RTE_RELATION:
 			case RTE_CTE:
+			case RTE_BEFORE:
 /* noth

Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])

[Stephen Frost]

* Tom Lane (t...@sss.pgh.pa.us) wrote:
> Stephen Frost  writes:
> > * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
> >> The conf.d/ path would be relative to postgresql.conf, so there's no
> >> need for Debian to patch anything.
> 
> > Uhhh, I really don't see that working, at all...
> 
> Why not?  conf.d is for installable files, AIUI.  What we need to
> be writable is auto.conf, but I thought we'd agreed that would live
> inside $PGDATA.

I agree that auto.conf should live in $PGDATA, but I really don't like
the idea of conf.d being relative to some other existing file.  It
should be included through an 'includedir' option, not just picked up as
some magic directory name, and therefore consider the current
arrangement of parameters in Debian:

data_directory = '/var/lib/postgresql/9.2/main'
hba_file = '/etc/postgresql/9.2/main/pg_hba.conf'
ident_file = '/etc/postgresql/9.2/main/pg_ident.conf'

and postgres is started like so:

/usr/lib/postgresql/9.2/bin/postgres -D /var/lib/postgresql/9.2/main -c 
config_file=/etc/postgresql/9.2/main/postgresql.conf

With the proposed include line for auto.conf, which lives in $PGDATA,
we'd have:

include 'auto.conf'

Would we then have

includedir 'conf.d'

which is relative to postgresql.conf instead?

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])

[Alvaro Herrera]

Stephen Frost escribió:
> * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
> > Stephen Frost escribió:

> > > I'd much rather have an includedir directive than some hard-coded or
> > > command-line option to read the directory..  The directory should live
> > > in /etc/postgresql/X.Y/cluster/ on at least Debian derivatives..
> > 
> > The conf.d/ path would be relative to postgresql.conf, so there's no
> > need for Debian to patch anything.
> 
> Uhhh, I really don't see that working, at all...

I don't understand why not.  Care to explain?

-- 
Álvaro Herrerahttp://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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])

[Stephen Frost]

* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
> Stephen Frost escribió:
> > * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
> > > Stephen Frost escribió:
> 
> > > > I'd much rather have an includedir directive than some hard-coded or
> > > > command-line option to read the directory..  The directory should live
> > > > in /etc/postgresql/X.Y/cluster/ on at least Debian derivatives..
> > > 
> > > The conf.d/ path would be relative to postgresql.conf, so there's no
> > > need for Debian to patch anything.
> > 
> > Uhhh, I really don't see that working, at all...
> 
> I don't understand why not.  Care to explain?

Tried to in my other mail, but let me also point out that we
("PGDG"/Upstream) don't "own" the directory in which postgresql.conf
lives.  At least on Debian and relatives, that directory isn't under
$PGDATA and it already has other files in it beyond postgresql.conf or
even the other PostgreSQL config files of hba and ident.  Here's the
default directory setup on Debian for /etc/postgresql/9.2/main/:

-rw-r--r-- 1 postgres postgres   316 Jun 29 22:07 environment
-rw-r--r-- 1 postgres postgres   143 Jun 29 22:07 pg_ctl.conf
-rw-r- 1 postgres postgres  4649 Jun 29 22:07 pg_hba.conf
-rw-r- 1 postgres postgres  1636 Jun 29 22:07 pg_ident.conf
-rw-r--r-- 1 postgres postgres 19770 Jun 29 22:07 postgresql.conf
-rw-r--r-- 1 postgres postgres   378 Jun 29 22:07 start.conf

There's three other files there and some sysadmins may have already
created their own 'conf.d' directory, perhaps to use for building the
postgresql.conf file or similar.  We must have a way to disable the
conf.d option and a way to name it something other than 'conf.d', imv.

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])

[Alvaro Herrera]

Stephen Frost escribió:

> I agree that auto.conf should live in $PGDATA, but I really don't like
> the idea of conf.d being relative to some other existing file.  It
> should be included through an 'includedir' option, not just picked up as
> some magic directory name, and therefore consider the current
> arrangement of parameters in Debian:
> 
> data_directory = '/var/lib/postgresql/9.2/main'
> hba_file = '/etc/postgresql/9.2/main/pg_hba.conf'
> ident_file = '/etc/postgresql/9.2/main/pg_ident.conf'
> 
> and postgres is started like so:
> 
> /usr/lib/postgresql/9.2/bin/postgres -D /var/lib/postgresql/9.2/main -c 
> config_file=/etc/postgresql/9.2/main/postgresql.conf
> 
> With the proposed include line for auto.conf, which lives in $PGDATA,
> we'd have:
> 
> include 'auto.conf'
> 
> Would we then have
> 
> includedir 'conf.d'
> 
> which is relative to postgresql.conf instead?

Well, all the relative paths in include/includedir directives would be
relative to the directory specified by the -c config_file param, which
makes perfect sense.  So the conf.d would work fine in your example.

The only problem I see in your snippet above is the "include auto.conf"
line, which doesn't make any sense because that file would not be found.
Hence my proposal that the file ought to be read automatically, not via
an include line.  Sadly I don't think we cannot just make it an absolute
path, in case the data dir is moved or whatever; the only choice I see
would be to have something like
include-data 'auto.conf'
or something like that which tells the system that the file is not in
the config dir but in the data dir instead.  A nearby comment could let
the user know about this file being in the data directory instead of the
config directory.

-- 
Álvaro Herrerahttp://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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])

[Alvaro Herrera]

Stephen Frost escribió:

> Tried to in my other mail,

Yep, got it and replied, thanks.

> but let me also point out that we
> ("PGDG"/Upstream) don't "own" the directory in which postgresql.conf
> lives.  At least on Debian and relatives, that directory isn't under
> $PGDATA and it already has other files in it beyond postgresql.conf or
> even the other PostgreSQL config files of hba and ident.  Here's the
> default directory setup on Debian for /etc/postgresql/9.2/main/:
> 
> -rw-r--r-- 1 postgres postgres   316 Jun 29 22:07 environment
> -rw-r--r-- 1 postgres postgres   143 Jun 29 22:07 pg_ctl.conf
> -rw-r- 1 postgres postgres  4649 Jun 29 22:07 pg_hba.conf
> -rw-r- 1 postgres postgres  1636 Jun 29 22:07 pg_ident.conf
> -rw-r--r-- 1 postgres postgres 19770 Jun 29 22:07 postgresql.conf
> -rw-r--r-- 1 postgres postgres   378 Jun 29 22:07 start.conf
> 
> There's three other files there and some sysadmins may have already
> created their own 'conf.d' directory, perhaps to use for building the
> postgresql.conf file or similar.  We must have a way to disable the
> conf.d option and a way to name it something other than 'conf.d', imv.

Uhm.  I find it hard to care much for this position.  Surely config
files are not migrated automatically from one major version to the next,
so if somebody created a 9.3/main/conf.d directory, they will have to
change it when they migrate to 9.4.

-- 
Álvaro Herrerahttp://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] CAST Within EXCLUSION constraint

[David E. Wheeler]

Hackers,

I am trying to do something like this:

CREATE TYPE source AS ENUM(
'fred', 'wilma', 'barney', 'betty'
);

CREATE EXTENSION btree_gist;

CREATE TABLE things (
source source NOT NULL,
within tstzrange NOT NULL,
EXCLUDE USING gist (source WITH =, within WITH &&)
);

Alas, enums are not supported by btree_gist:

try.sql:13: ERROR:  data type source has no default operator class for 
access method "gist"
HINT:  You must specify an operator class for the index or define a default 
operator class for the data type.

Well, maybe I can cast it? But no, changing the EXCLUDE line to

EXCLUDE USING gist (source::text WITH =, within WITH &&)

Yields a syntax error:

try.sql:13: ERROR:  syntax error at or near "::"
LINE 4: EXCLUDE USING gist (source::text WITH =, within WITH &&)

So that's out. Why shouldn't :: be allowed?

No problem, I can use CAST(), right? So I try:

EXCLUDE USING gist (CAST(source AS text) WITH =, within WITH &&)

Not so much:

try.sql:13: ERROR:  functions in index expression must be marked IMMUTABLE

I guess it's because locale settings might change, and therefore change the 
text representation? Seems unlikely, though.

I guess I can create my own IMMUTABLE function over the ENUM:

CREATE FUNCTION source_to_text(
source
) RETURNS TEXT LANGUAGE sql STRICT IMMUTABLE AS $$
SELECT $1::text;
$$;

So this works:

EXCLUDE USING gist (source_to_text(source) WITH =, within WITH &&)

So I guess that’s good enough for now. But should :: really be a syntax error 
in index expressions?

Thanks,

David

-- 
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] ereport documentation patch

[Christophe Pettus]

On Aug 19, 2013, at 11:28 PM, Heikki Linnakangas wrote:

> On 19.08.2013 23:40, Christophe Pettus wrote:
>> Is it reasonable to note in the documentation that ereport does not return 
>> if the error severity is greater than or equal to ERROR?
> 
> Yeah, it probably would be good to mention that. Got a patch?

Attached!


ereport-no-return-doc.patch
Description: Binary data



--
-- Christophe Pettus
   x...@thebuild.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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])

[Stephen Frost]

* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
> Well, all the relative paths in include/includedir directives would be
> relative to the directory specified by the -c config_file param, which
> makes perfect sense.  So the conf.d would work fine in your example.

Why would include/includedir directives be relative to where the
'config_file' GUC is set to instead of relative to where all the other
GUCs in postgresql.conf are relative to?  That is a recipe for
confusion, imv.

Of course, the current situation is quite terrible anyway, imv.  Having
the GUCs be relative to whereever the user happens to run pg_ctl from is
pretty ugly- not to mention that the commented out 'defaults' won't
actually work if you uncomment them because the *actual* default/unset
value *is* in the data directory.  I'm starting to wish for a way to do
variable substitution inside postgresql.conf, so we could have defaults
that would actually work if uncommented (eg: $DataDir/pg_hba.conf).

That would help with auto.conf also.

> The only problem I see in your snippet above is the "include auto.conf"
> line, which doesn't make any sense because that file would not be found.

To be honest, I was considering 'include' to be relative to the data
directory and handled similar to how we process recovery.conf, but as we
consider paths in postgresql.conf to be relative to $PWD, that's not
ideal because it'd be different from the rest of the file references.

In any case, while the current situation sucks, I don't think we can go
changing how relative files are treated in postgresql.conf, nor should
we make the way a path is processed change depending on which GUC is
being set.

While I really like the 'include auto.conf' style, I'm starting to think
it may not be workable after all.  Another thing to consider is if the
user decides to change that include line..  What happens when the DBA
tries to do a 'ALTER SYSTEM'?  It'd still use the hard-coded auto.conf
file and happily update it, I imagine, but it won't actually get
included...

Thanks,

Stephen


signature.asc
Description: Digital signature

[HACKERS] Detail part for still waiting for lock log message

[Tarvi Pillessaar]

Hello!

From time to time when investigating different locking issues using 
postgres log i have thought that "process x is still waiting for" 
message could be more informative, for example at the moment it is quite 
painful to find out which session was actually holding that particular lock.


I would like to add detail part for this message to show information 
about the lock holder and also show what the lock holder is actually 
doing (yes, i know that i could get the lock holder's statement from the 
log, but this not the same, maybe lock holder was idle in transaction).
So, i wrote a small patch, but i am not sure that this is the 
best/correct way to do it.

Maybe someone can take a look at my patch and give some feedback.
Even if this idea won't reach to upstream, i would still like to get 
feedback.


About patch:
Patch is tested against 9.2.4.
I was not sure that i should check if the lock holder's proclock was 
found (as lock holder's proclock should be always there), check is there 
to be on the safe side, but maybe it's unnecessary.
If it's not needed then fallback to old behavior (logging without 
detail) is not needed as well.
And yes, i know that the lock holding time is not actually correct and 
it actually shows milliseconds since transaction start.


Regards,
Tarvi Pillessaar
diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c
index 3bfdd23..4dc7b37 100644
--- a/src/backend/storage/lmgr/proc.c
+++ b/src/backend/storage/lmgr/proc.c
@@ -49,6 +49,7 @@
 #include "storage/procsignal.h"
 #include "storage/spin.h"
 #include "utils/timestamp.h"
+#include "pgstat.h"
 
 
 /* GUC variables */
@@ -1198,9 +1199,66 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable)
 			}
 
 			if (myWaitStatus == STATUS_WAITING)
-ereport(LOG,
-		(errmsg("process %d still waiting for %s on %s after %ld.%03d ms",
-			  MyProcPid, modename, buf.data, msecs, usecs)));
+			{
+PROCLOCK   *proclock2;
+PGPROC *proc2;
+PgBackendStatus *be;
+bool found = false;
+
+/* find lock holder */
+proclock2 = (PROCLOCK *) SHMQueueNext(&(lock->procLocks), &(lock->procLocks),
+		offsetof(PROCLOCK, lockLink));
+while (proclock2)
+{
+	if (lockMethodTable->conflictTab[lockmode] & proclock2->holdMask)
+		break;
+
+	proclock2 = (PROCLOCK *) SHMQueueNext(&(lock->procLocks), &proclock2->lockLink,
+			offsetof(PROCLOCK, lockLink));
+}
+
+if (proclock2)
+{
+	proc2 = proclock2->tag.myProc;
+
+	/* get lock holder's beentry */
+	int numbackends = pgstat_fetch_stat_numbackends();
+	for (i = 1; i <= numbackends; i++)
+	{
+		be = pgstat_fetch_stat_beentry(i);
+		if (be)
+		{
+			if (be->st_procpid == proc2->pid)
+			{
+found = true;
+break;
+			}
+		}
+	}
+}
+
+if (found)
+{
+	long secs2;
+	int usecs2;
+	long msecs2;
+
+	/* calculate lock holder's tx duration */
+	TimestampDifference(be->st_xact_start_timestamp, GetCurrentTimestamp(), &secs2, &usecs2);
+	msecs2 = secs2 * 1000 + usecs2 / 1000;
+	usecs2 = usecs2 % 1000;
+
+	ereport(LOG,
+			(errmsg("process %d still waiting for %s on %s after %ld.%03d ms",
+	MyProcPid, modename, buf.data, msecs, usecs),
+			 errdetail_log("process %d is holding lock for %ld.%03d ms, activity: %s",
+	proc2->pid, msecs2, usecs2, be->st_activity)));
+}
+else
+	ereport(LOG,
+			(errmsg("process %d still waiting for %s on %s after %ld.%03d ms",
+	MyProcPid, modename, buf.data, msecs, usecs)));
+			}
 			else if (myWaitStatus == STATUS_OK)
 ereport(LOG,
 	(errmsg("process %d acquired %s on %s after %ld.%03d ms",

-- 
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] Personal note: taking some vacation time in Sep/Oct

[Steve Crawford]

On 08/19/2013 11:55 PM, Gavin Flower wrote:

On 20/08/13 15:26, Tom Lane wrote:

I will be taking a long (and long-overdue) vacation...
but, But, BUT, you're not human - you can't possibly take leave, the 
sky will fall & all manners of divers calamities will come to pass!!!


As if on cue: 
http://www.nasa.gov/content/goddard/the-suns-magnetic-field-is-about-to-flip/


Cheers,
Steve

Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])

[Alvaro Herrera]

Stephen Frost escribió:
> * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
> > Well, all the relative paths in include/includedir directives would be
> > relative to the directory specified by the -c config_file param, which
> > makes perfect sense.  So the conf.d would work fine in your example.
> 
> Why would include/includedir directives be relative to where the
> 'config_file' GUC is set to instead of relative to where all the other
> GUCs in postgresql.conf are relative to?  That is a recipe for
> confusion, imv.
> 
> Of course, the current situation is quite terrible anyway, imv.  Having
> the GUCs be relative to whereever the user happens to run pg_ctl from is
> pretty ugly- not to mention that the commented out 'defaults' won't
> actually work if you uncomment them because the *actual* default/unset
> value *is* in the data directory.

Uh?  See the docs:
http://www.postgresql.org/docs/devel/static/config-setting.html#CONFIG-INCLUDES

 " ... the postgresql.conf file can contain include directives, ...
  If the file name is not an absolute path, it is taken as relative to
  the directory containing the referencing configuration file."


> I'm starting to wish for a way to do
> variable substitution inside postgresql.conf, so we could have defaults
> that would actually work if uncommented (eg: $DataDir/pg_hba.conf).
> 
> That would help with auto.conf also.

Grumble.  I don't think we need this.  At least, not for ALTER SYSTEM or
conf.d.

> > The only problem I see in your snippet above is the "include auto.conf"
> > line, which doesn't make any sense because that file would not be found.
> 
> To be honest, I was considering 'include' to be relative to the data
> directory and handled similar to how we process recovery.conf,

Well, recovery.conf is a special case that doesn't follow to normal
rules.

> but as we
> consider paths in postgresql.conf to be relative to $PWD, that's not
> ideal because it'd be different from the rest of the file references.

I don't know where you got that idea from, but it's wrong.

> While I really like the 'include auto.conf' style, I'm starting to think
> it may not be workable after all.  Another thing to consider is if the
> user decides to change that include line..  What happens when the DBA
> tries to do a 'ALTER SYSTEM'?  It'd still use the hard-coded auto.conf
> file and happily update it, I imagine, but it won't actually get
> included...

Well, this whole line of discussion started because I objected to the
whole code path that was trying to detect whether auto.conf had been
parsed, and raised a warning if ALTER SYSTEM was executed and the file
wasn't parsed.

-- 
Álvaro Herrerahttp://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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])

[Tom Lane]

Alvaro Herrera  writes:
> Stephen Frost escribió:
>> While I really like the 'include auto.conf' style, I'm starting to think
>> it may not be workable after all.  Another thing to consider is if the
>> user decides to change that include line..  What happens when the DBA
>> tries to do a 'ALTER SYSTEM'?  It'd still use the hard-coded auto.conf
>> file and happily update it, I imagine, but it won't actually get
>> included...

> Well, this whole line of discussion started because I objected to the
> whole code path that was trying to detect whether auto.conf had been
> parsed, and raised a warning if ALTER SYSTEM was executed and the file
> wasn't parsed.

I really, really don't think that we should be trying to detect or prevent
any such thing.  If the user breaks it like that, he gets to keep both
pieces --- and who's to say it's broken, anyway?  Disabling ALTER SYSTEM
might have been exactly his intent.

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] Personal note: taking some vacation time in Sep/Oct

[Christopher Browne]

On Tue, Aug 20, 2013 at 2:55 AM, Gavin Flower
 wrote:
> On 20/08/13 15:26, Tom Lane wrote:
>
> I will be taking a long (and long-overdue) vacation from Sep 10 to Oct 20.
> I expect to have email access, but won't be doing much more than minimally
> keeping up with my inbox.
>
> This means I'll be pretty much AWOL for the September commitfest :-(.
> That's unfortunate, but the dates for this trip were frozen long before
> the 9.4 development calendar was.
>
> regards, tom lane
>
>
> but, But, BUT, you're not human - you can't possibly take leave, the sky
> will fall & all manners of divers calamities will come to pass!!!

I think a scene from Ghostbusters comes in handy here...

Dr. Peter Venkman: This city is headed for a disaster of biblical proportions.
Mayor: What do you mean, "biblical"?
Dr Ray Stantz: What he means is Old Testament, Mr. Mayor, real wrath
of God type stuff.
Dr. Peter Venkman: Exactly.
Dr Ray Stantz: Fire and brimstone coming down from the skies! Rivers
and seas boiling!
Dr. Egon Spengler: Forty years of darkness! Earthquakes, volcanoes...
Winston Zeddemore: The dead rising from the grave!
Dr. Peter Venkman: Human sacrifice, dogs and cats living together...
mass hysteria!
Mayor: All right, all right! I get the point!

Man, dogs and cats living together!!!

[I wonder what skewing this will have on the analytical statistics on
the pgsql mailing lists???
]

> MORE SERIOUSLY:
> Enjoy your more than well earnt leave!

Indeed.  We'll try to keep the dogs and cats suitably apart!  :-)
-- 
When confronted by a difficult problem, solve it by reducing it to the
question, "How would the Lone Ranger handle this?"


-- 
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] CAST Within EXCLUSION constraint

[Tom Lane]

"David E. Wheeler"  writes:
> Well, maybe I can cast it? But no, changing the EXCLUDE line to
> EXCLUDE USING gist (source::text WITH =, within WITH &&)
> Yields a syntax error:
> try.sql:13: ERROR:  syntax error at or near "::"
> LINE 4: EXCLUDE USING gist (source::text WITH =, within WITH &&)
> So that's out. Why shouldn't :: be allowed?

You need more parentheses -- (source::text) would've worked.

> No problem, I can use CAST(), right? So I try:
> EXCLUDE USING gist (CAST(source AS text) WITH =, within WITH &&)
> Not so much:
> try.sql:13: ERROR:  functions in index expression must be marked IMMUTABLE
> I guess it's because locale settings might change, and therefore change the 
> text representation? Seems unlikely, though.

Not locale, just renaming one of the values would be enough to break that.
Admittedly we don't provide an official way to do that ATM, but you can do
an UPDATE on pg_enum.

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] CAST Within EXCLUSION constraint

[David E. Wheeler]

On Aug 20, 2013, at 6:50 PM, Tom Lane  wrote:

> You need more parentheses -- (source::text) would've worked.

Alas, no, same problem as for CAST():

  ERROR:  functions in index expression must be marked IMMUTABLE

>> No problem, I can use CAST(), right? So I try:
>>EXCLUDE USING gist (CAST(source AS text) WITH =, within WITH &&)
>> Not so much:
>>try.sql:13: ERROR:  functions in index expression must be marked IMMUTABLE
>> I guess it's because locale settings might change, and therefore change the 
>> text representation? Seems unlikely, though.
> 
> Not locale, just renaming one of the values would be enough to break that.
> Admittedly we don't provide an official way to do that ATM, but you can do
> an UPDATE on pg_enum.

Ah, right. Maybe if there was a way to get at some immutable numeric value…

Thanks,

David



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

Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])

[Stephen Frost]

* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
> Stephen Frost escribió:
> > Of course, the current situation is quite terrible anyway, imv.  Having
> > the GUCs be relative to whereever the user happens to run pg_ctl from is
> > pretty ugly- not to mention that the commented out 'defaults' won't
> > actually work if you uncomment them because the *actual* default/unset
> > value *is* in the data directory.
> 
> Uh?  See the docs:
> http://www.postgresql.org/docs/devel/static/config-setting.html#CONFIG-INCLUDES
> 
>  " ... the postgresql.conf file can contain include directives, ...
>   If the file name is not an absolute path, it is taken as relative to
>   the directory containing the referencing configuration file."

And what about

http://www.postgresql.org/docs/9.3/static/runtime-config-file-locations.html

 ... ?

   "When setting any of these parameters, a relative path will be
   interpreted with respect to the directory in which postgres is
   started."

> > I'm starting to wish for a way to do
> > variable substitution inside postgresql.conf, so we could have defaults
> > that would actually work if uncommented (eg: $DataDir/pg_hba.conf).
> > 
> > That would help with auto.conf also.
> 
> Grumble.  I don't think we need this.  At least, not for ALTER SYSTEM or
> conf.d.

imv, it would be handy.  Along with a $ConfDir which is the
postgresql.conf location- it would certainly make things clearer about
what files are being included from where.

> > To be honest, I was considering 'include' to be relative to the data
> > directory and handled similar to how we process recovery.conf,
> 
> Well, recovery.conf is a special case that doesn't follow to normal
> rules.

I don't see why it should be.

> > but as we
> > consider paths in postgresql.conf to be relative to $PWD, that's not
> > ideal because it'd be different from the rest of the file references.
> 
> I don't know where you got that idea from, but it's wrong.

Not sure which you're referring to, but see above from the docs?  Also,
please tias..  I was amazed that we use $PWD also, but we do..

> Well, this whole line of discussion started because I objected to the
> whole code path that was trying to detect whether auto.conf had been
> parsed, and raised a warning if ALTER SYSTEM was executed and the file
> wasn't parsed.

I like the idea that we complain if ALTER SYSTEM ends up being
idempotent..  Not sure how far we should take that, but accepting
commands which end up doing nothing is bad, imv.

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] Detail part for still waiting for lock log message

[Kevin Grittner]

Tarvi Pillessaar  wrote:

> Maybe someone can take a look at my patch and give some feedback.

> Even if this idea won't reach to upstream, i would still like to get 
> feedback.

Please add the patch to the open CommitFest.

You can read about the process here:

http://wiki.postgresql.org/wiki/CommitFest


--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



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

Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])

[Alvaro Herrera]

Stephen Frost escribió:

> And what about
> 
> http://www.postgresql.org/docs/9.3/static/runtime-config-file-locations.html
> 
>  ... ?
> 
>"When setting any of these parameters, a relative path will be
>interpreted with respect to the directory in which postgres is
>started."

That's talking about the data_directory and the various foo_file
settings, though; it doesn't apply to the include settings.  Note
especially that config_file says it can only be set on the postgres
command line.  (I think a saner definition would have been to state that
relative paths are not allowed in the command line.  But that ship has
already sailed.  And relative paths seem useful in the config file; and
maintaining the distinction that they are allowed within the config file
but not in the command line might be awkward.)


(Uhm, when a command line contains stuff, is the stuff "in" the command
line or "on" it?)

-- 
Álvaro Herrerahttp://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] GSOC13 proposal - extend RETURNING syntax

[Boszormenyi Zoltan]

2013-08-20 17:30 keltezéssel, Karol Trzcionka írta:

W dniu 20.08.2013 16:47, Karol Trzcionka pisze:

Thank you for the review and tests. New version introduce a lot of
improvements:
- Fix regression test for view (wrong table_name)
- Add regression test for inheritance
- Delete hack in initsplan.c (now we ignore all RTE_BEFORE) - the
uninitialized issue
- Revert changing varno in add_vars_to_targetlist
- Add all "before" variables to targetlist
- Avoid adding variables to slot for AFTER.
- Treat varnoold like a flag - prevent from adjustment if RTE_BEFORE
- All before/after are now set on OUTER_VAR
- Rename fix_varno_varattno to bind_returning_variables
- Add comment about bind_returning_variables
- Remove unneeded code in fix_join_expr_mutator (it was changing varno
 of RTE_BEFORE - now there is not any var with varno assigned to it)

I've just realized the prepare_returning_before() is unneeded right now
so I've removed it. Version 7, hopefully the last. ;)


Here's a new one, for v7:

setrefs.c: In function ‘set_plan_refs’:
setrefs.c:2001:26: warning: ‘before_index’ may be used uninitialized in this function 
[-Wmaybe-uninitialized]

  bind_returning_variables(rlist, before_index, after_index);
  ^
setrefs.c:1957:21: note: ‘before_index’ was declared here
  int after_index=0, before_index;
 ^

Best regards,
Zoltán Böszörményi


Regards,
Karol Trzcionka





--
--
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt, Austria
Web: http://www.postgresql-support.de
 http://www.postgresql.at/

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

[Karol Trzcionka]

W dniu 20.08.2013 20:55, Boszormenyi Zoltan pisze:
> Here's a new one, for v7:
>
> setrefs.c: In function ‘set_plan_refs’:
> setrefs.c:2001:26: warning: ‘before_index’ may be used uninitialized
> in this function [-Wmaybe-uninitialized]
>   bind_returning_variables(rlist, before_index, after_index);
>   ^
> setrefs.c:1957:21: note: ‘before_index’ was declared here
>   int after_index=0, before_index;
>  ^
Right, my mistake. Sorry and thanks. Fixed.
Regards,
Karol Trzcionka
diff --git a/doc/src/sgml/ref/update.sgml b/doc/src/sgml/ref/update.sgml
index 90b9208..eba35f0 100644
--- a/doc/src/sgml/ref/update.sgml
+++ b/doc/src/sgml/ref/update.sgml
@@ -194,12 +194,27 @@ UPDATE [ ONLY ] table_name [ * ] [
 output_expression
 
  
-  An expression to be computed and returned by the UPDATE
-  command after each row is updated.  The expression can use any
-  column names of the table named by table_name
-  or table(s) listed in FROM.
-  Write * to return all columns.
+  An expression to be computed and returned by the
+  UPDATE command either before or after (prefixed with
+  BEFORE. and AFTER.,
+  respectively) each row is updated.  The expression can use any
+  column names of the table named by table_name or table(s) listed in
+  FROM.  Write AFTER.* to return all 
+  columns after the update. Write BEFORE.* for all
+  columns before the update. Write * to return all
+  columns after update and all triggers fired (these values are in table
+  after command). You may combine BEFORE, AFTER and raw columns in the
+  expression.
  
+ 
+ Mixing table names or aliases named before or after with the
+ above will result in confusion and suffering.  If you happen to
+ have a table called before or
+ after, alias it to something else when using
+ RETURNING.
+ 
+
 

 
@@ -287,15 +302,16 @@ UPDATE weather SET temp_lo = temp_lo+1, temp_hi = temp_lo+15, prcp = DEFAULT
   
 
   
-   Perform the same operation and return the updated entries:
+   Perform the same operation and return information on the changed entries:
 
 
 UPDATE weather SET temp_lo = temp_lo+1, temp_hi = temp_lo+15, prcp = DEFAULT
   WHERE city = 'San Francisco' AND date = '2003-07-03'
-  RETURNING temp_lo, temp_hi, prcp;
+  RETURNING temp_lo AS new_low, temp_hi AS new_high, BEFORE.temp_hi/BEFORE.temp_low AS old_ratio, AFTER.temp_hi/AFTER.temp_low AS new_ratio prcp;
 
   
 
+
   
Use the alternative column-list syntax to do the same update:
 
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index d86e9ad..fafd311 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -2335,7 +2335,7 @@ ExecASUpdateTriggers(EState *estate, ResultRelInfo *relinfo)
 TupleTableSlot *
 ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
 	 ResultRelInfo *relinfo,
-	 ItemPointer tupleid, TupleTableSlot *slot)
+	 ItemPointer tupleid, TupleTableSlot *slot, TupleTableSlot **planSlot)
 {
 	TriggerDesc *trigdesc = relinfo->ri_TrigDesc;
 	HeapTuple	slottuple = ExecMaterializeSlot(slot);
@@ -2381,6 +2381,7 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
 	if (newSlot != NULL)
 	{
 		slot = ExecFilterJunk(relinfo->ri_junkFilter, newSlot);
+		*planSlot = newSlot;
 		slottuple = ExecMaterializeSlot(slot);
 		newtuple = slottuple;
 	}
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 15f5dcc..06ebaf3 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -603,7 +603,7 @@ ExecUpdate(ItemPointer tupleid,
 		resultRelInfo->ri_TrigDesc->trig_update_before_row)
 	{
 		slot = ExecBRUpdateTriggers(estate, epqstate, resultRelInfo,
-	tupleid, slot);
+	tupleid, slot, &planSlot);
 
 		if (slot == NULL)		/* "do nothing" */
 			return NULL;
@@ -737,6 +737,7 @@ lreplace:;
 		   hufd.xmax);
 	if (!TupIsNull(epqslot))
 	{
+		planSlot = epqslot;
 		*tupleid = hufd.ctid;
 		slot = ExecFilterJunk(resultRelInfo->ri_junkFilter, epqslot);
 		tuple = ExecMaterializeSlot(slot);
diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c
index 908f397..461ec4f 100644
--- a/src/backend/nodes/nodeFuncs.c
+++ b/src/backend/nodes/nodeFuncs.c
@@ -1987,6 +1987,7 @@ range_table_walker(List *rtable,
 		{
 			case RTE_RELATION:
 			case RTE_CTE:
+			case RTE_BEFORE:
 /* nothing to do */
 break;
 			case RTE_SUBQUERY:
@@ -2701,6 +2702,7 @@ range_table_mutator(List *rtable,
 		{
 			case RTE_RELATION:
 			case RTE_CTE:
+			case RTE_BEFORE:
 /* we don't bother to copy eref, aliases, etc; OK? */
 break;
 			case RTE_SUBQUERY:
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index cff4734..3c4e045 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -2368,6 +2368,7 @@ 

Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])

[Stephen Frost]

* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
> Stephen Frost escribió:
> > http://www.postgresql.org/docs/9.3/static/runtime-config-file-locations.html
> 
> That's talking about the data_directory and the various foo_file
> settings, though; it doesn't apply to the include settings.  

Right- that's what I'm bitching about.  We have various references to
file locations, with defined handling of relative locations, and the
'include' system completely ignores that and instead invents its own
making the result a confusing mess.

> Note
> especially that config_file says it can only be set on the postgres
> command line.  (I think a saner definition would have been to state that
> relative paths are not allowed in the command line.  But that ship has
> already sailed.  And relative paths seem useful in the config file; and
> maintaining the distinction that they are allowed within the config file
> but not in the command line might be awkward.)

Relative paths based on $PWD are useful?  Really?  Not on my systems
anyway..

> (Uhm, when a command line contains stuff, is the stuff "in" the command
> line or "on" it?)

A just question- I vote 'on'. :)

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])

[Martijn van Oosterhout]

On Tue, Aug 20, 2013 at 11:23:06AM -0400, Stephen Frost wrote:
> > What I was proposing upthread is that enable_alter_system=off/on would
> > be present in postgresql.conf, and there is no include line for
> > auto.conf.  
> 
> I really think that's a terrible approach, to be honest.  I want to see
> an 'include' line in postgresql.conf for auto.conf, so the hapless
> sysadmin who is trying to figure out what the crazy DBA did has some
> clue what to look for.  "enable_alter_system" doesn't tell him diddly
> about an 'auto.conf' file which is included in the system config.

ISTM you want some kind of hybrid setting like:

#include_system auto.conf

which simultaneously does three things:

1. Sets the enable_alter_system flag
2. Indicates the file to use
3. Indicates the priority of the setting re other settings.

Comment it out, ALTER SYSTEM stop working. Put it back and it's
immediately clear what it means. And the user can control where the
settings go.

Syntax is a bit fugly though.

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

[HACKERS] Back-patch change in hashed DISTINCT estimation?

[Tom Lane]

In a thread over in pgsql-performance, Tomas Vondra pointed out that
choose_hashed_distinct was sometimes making different choices than
choose_hashed_grouping, so that queries like these:

select distinct x from ... ;
select x from ... group by 1;

might get different plans even though they should be equivalent.
After some debugging it turns out that I omitted hash_agg_entry_size()
from the hash table size estimate in choose_hashed_distinct --- I'm pretty
sure I momentarily thought that this function must yield zero if there are
no aggregates, but that's wrong.  So we need a patch like this:

diff --git a/src/backend/optimizer/plan/planner.c 
b/src/backend/optimizer/plan/planner.c
index bcc0d45..99284cb 100644
*** a/src/backend/optimizer/plan/planner.c
--- b/src/backend/optimizer/plan/planner.c
*** choose_hashed_distinct(PlannerInfo *root
*** 2848,2854 
--- 2848,2858 
 * Don't do it if it doesn't look like the hashtable will fit into
 * work_mem.
 */
+ 
+   /* Estimate per-hash-entry space at tuple width... */
hashentrysize = MAXALIGN(path_width) + 
MAXALIGN(sizeof(MinimalTupleData));
+   /* plus the per-hash-entry overhead */
+   hashentrysize += hash_agg_entry_size(0);
  
if (hashentrysize * dNumDistinctRows > work_mem * 1024L)
return false;

When grouping narrow data, like a float or a couple of ints, this
oversight makes for more than 2X error in the hash table size estimate.

What I'm wondering is whether to back-patch this or leave well enough
alone.  The risk of back-patching is that it might destabilize plan
choices that people like.  (In Tomas' original example, the underestimate
of the table size leads it to choose a plan that is in fact better.)
The risk of not back-patching is that the error could lead to
out-of-memory failures because the hash aggregation uses more memory
than the planner expected.  (Tomas was rather fortunate in that his
case had an overestimate of dNumDistinctRows, so it didn't end up
blowing out memory ... but usually I think we underestimate that more
than overestimate it.)

A possible compromise is to back-patch into 9.3 (where the argument about
destabilizing plan choices doesn't have much force yet), but not further.

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] danger of stats_temp_directory = /dev/shm

[Alvaro Herrera]

Tom Lane wrote:
> Andres Freund  writes:
> > On 2013-08-19 14:28:28 -0400, Tom Lane wrote:
> >> One possibility is to do the initial check somewhere shortly after
> >> ChangeToDataDir(), and have the GUC check hook only attempt to make a
> >> check in SIGHUP context.  Unfortunately we aren't passing the context to
> >> check hooks, only GucSource which isn't adequate for this.  Not sure if we
> >> want to go so far as to change the check-hook API at this point.  We could
> >> probably think of some other, klugy way to tell if it's initial startup.
> 
> > Is it even actually safe to have stats_temp_directory PGC_SIGHUP after
> > the per DB splitup? I haven't audited the code for it, but it seems
> > somewhat likely that we would end up with some files in the old and some
> > in the new directory?
> 
> That's a good point.  For the moment we could just change it to
> PGC_POSTMASTER and eliminate this problem.  Reducing it back to SIGHUP
> would be a future feature, which is evidently less than trivial.

Here's a patchset for this.  The first patch is the same as upthread,
with the enum members renamed; the second is the actual pgstats change.

(I considered the idea that checkDirectoryPermissions returned a bitmask
of the failed checks, instead of just returning a code for the first
check that fails.  This might be useful if some caller wants to ignore
certain problems.  But at the moment I didn't see many other places
where this could be used, so it's probably pointless ATM.)

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
*** a/src/backend/postmaster/postmaster.c
--- b/src/backend/postmaster/postmaster.c
***
*** 1313,1385  getInstallationPaths(const char *argv0)
  static void
  checkDataDir(void)
  {
  	char		path[MAXPGPATH];
  	FILE	   *fp;
- 	struct stat stat_buf;
  
  	Assert(DataDir);
  
! 	if (stat(DataDir, &stat_buf) != 0)
  	{
! 		if (errno == ENOENT)
  			ereport(FATAL,
  	(errcode_for_file_access(),
  	 errmsg("data directory \"%s\" does not exist",
  			DataDir)));
! 		else
  			ereport(FATAL,
  	(errcode_for_file_access(),
!  errmsg("could not read permissions of directory \"%s\": %m",
! 		DataDir)));
  	}
  
- 	/* eventual chdir would fail anyway, but let's test ... */
- 	if (!S_ISDIR(stat_buf.st_mode))
- 		ereport(FATAL,
- (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-  errmsg("specified data directory \"%s\" is not a directory",
- 		DataDir)));
- 
- 	/*
- 	 * Check that the directory belongs to my userid; if not, reject.
- 	 *
- 	 * This check is an essential part of the interlock that prevents two
- 	 * postmasters from starting in the same directory (see CreateLockFile()).
- 	 * Do not remove or weaken it.
- 	 *
- 	 * XXX can we safely enable this check on Windows?
- 	 */
- #if !defined(WIN32) && !defined(__CYGWIN__)
- 	if (stat_buf.st_uid != geteuid())
- 		ereport(FATAL,
- (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-  errmsg("data directory \"%s\" has wrong ownership",
- 		DataDir),
-  errhint("The server must be started by the user that owns the data directory.")));
- #endif
- 
- 	/*
- 	 * Check if the directory has group or world access.  If so, reject.
- 	 *
- 	 * It would be possible to allow weaker constraints (for example, allow
- 	 * group access) but we cannot make a general assumption that that is
- 	 * okay; for example there are platforms where nearly all users
- 	 * customarily belong to the same group.  Perhaps this test should be
- 	 * configurable.
- 	 *
- 	 * XXX temporarily suppress check when on Windows, because there may not
- 	 * be proper support for Unix-y file permissions.  Need to think of a
- 	 * reasonable check to apply on Windows.
- 	 */
- #if !defined(WIN32) && !defined(__CYGWIN__)
- 	if (stat_buf.st_mode & (S_IRWXG | S_IRWXO))
- 		ereport(FATAL,
- (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-  errmsg("data directory \"%s\" has group or world access",
- 		DataDir),
-  errdetail("Permissions should be u=rwx (0700).")));
- #endif
- 
  	/* Look for PG_VERSION before looking for pg_control */
  	ValidatePgVersion(DataDir);
  
--- 1313,1363 
  static void
  checkDataDir(void)
  {
+ 	CheckDirErrcode	err;
  	char		path[MAXPGPATH];
  	FILE	   *fp;
  
  	Assert(DataDir);
  
! 	err = checkDirectoryPermissions(DataDir);
! 	switch (err)
  	{
! 		case CKDIR_OK:
! 			break;
! 		case CKDIR_NOT_EXISTS:
  			ereport(FATAL,
  	(errcode_for_file_access(),
  	 errmsg("data directory \"%s\" does not exist",
  			DataDir)));
! 			break;
! 		case CKDIR_CANT_READ_PERMS:
  			ereport(FATAL,
  	(errcode_for_file_access(),
! 	 errmsg("could not read permissions of data directory \"%s\": %m",
! 			DataDir)));
! 			break;
! 		case CKDIR_NOT_DIR:
! 			ereport(FATAL,
! 	(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
! 	 errmsg("specified data directory \"%s\" is not a direc

Re: [HACKERS] 9.4 regression

[Bruce Momjian]

On Mon, Aug 19, 2013 at 09:27:57PM -0400, Stephen Frost wrote:
> * Andres Freund (and...@2ndquadrant.com) wrote:
> > I vote for adapting the patch to additionally zero out the file via
> > write(). In your tests that seemed to perform at least as good as the
> > old method... It also has the advantage that we can use it a littlebit
> > more as a testbed for possibly using it for heap extensions one day.
> > We're pretty early in the cycle, so I am not worried about this too much...
> 
> I dunno, I'm pretty disappointed that this doesn't actually improve
> things.  Just following this casually, it looks like it might be some
> kind of locking issue in the kernel that's causing it to be slower; or
> at least some code path that isn't exercise terribly much and therefore
> hasn't been given the love that it should.
> 
> Definitely interested in what Ts'o says, but if we can't figure out why
> it's slower *without* writing out the zeros, I'd say we punt on this
> until Linux and the other OS folks improve the situation.

Agreed.  Anyone with an affected kernel really can't be doing
performance tests right now, and that isn't good.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +


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

Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])

[Bruce Momjian]

On Tue, Aug 20, 2013 at 08:38:52AM +0530, Amit Kapila wrote:
> On Tue, Aug 20, 2013 at 8:27 AM, Stephen Frost  wrote:
> > * Amit Kapila (amit.kapil...@gmail.com) wrote:
> >> If both are okay, then I would like to go with second option (include
> >> file mechanism).
> >> I think by default, we should allow auto file to be included and if
> >> user faces any problem or otherwise,
> >> then he can decide to disable it.
> >
> > If it's enabled by default, then we need to ship an 'auto' file which is
> > empty by default...
> 
> initdb will create the empty auto file. If we don't enable by default,
> then if user uses
> ALTER SYSTEM and do sighup/restart, changed config parameter values
> will not come into affect
> until he manually enables it by removing '#' from '#include'.

Just a heads-up, but a lot of C language folks are going to look at:

#include abc

and think that is enabled.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +


-- 
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] Back-patch change in hashed DISTINCT estimation?

[Pavan Deolasee]

On Wed, Aug 21, 2013 at 2:54 AM, Tom Lane  wrote:

>
> What I'm wondering is whether to back-patch this or leave well enough
> alone.  The risk of back-patching is that it might destabilize plan
> choices that people like.  (In Tomas' original example, the underestimate
> of the table size leads it to choose a plan that is in fact better.)
> The risk of not back-patching is that the error could lead to
> out-of-memory failures because the hash aggregation uses more memory
> than the planner expected.


FWIW I recently investigated an out-of-memory issue in hash aggregation.
That case was because of use of a large temp table which was not manually
analysed and thus lead to a bad plan selection. But out of memory errors
are very confusing to the users and I have seen them unnecessarily
tinkering their memory settings to circumvent that issue. So +1 to fix the
bug in back branches, even though I understand there could be some
casualties on the border.

Thanks,
Pavan

-- 
Pavan Deolasee
http://www.linkedin.com/in/pavandeolasee

Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])

[Amit Kapila]

On Tue, Aug 20, 2013 at 10:02 PM, Alvaro Herrera
 wrote:
> Stephen Frost escribió:
>> * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
>> > Well, all the relative paths in include/includedir directives would be
>> > relative to the directory specified by the -c config_file param, which
>> > makes perfect sense.  So the conf.d would work fine in your example.
>>
>> Why would include/includedir directives be relative to where the
>> 'config_file' GUC is set to instead of relative to where all the other
>> GUCs in postgresql.conf are relative to?  That is a recipe for
>> confusion, imv.
>>
>> Of course, the current situation is quite terrible anyway, imv.  Having
>> the GUCs be relative to whereever the user happens to run pg_ctl from is
>> pretty ugly- not to mention that the commented out 'defaults' won't
>> actually work if you uncomment them because the *actual* default/unset
>> value *is* in the data directory.
>
> Uh?  See the docs:
> http://www.postgresql.org/docs/devel/static/config-setting.html#CONFIG-INCLUDES
>
>  " ... the postgresql.conf file can contain include directives, ...
>   If the file name is not an absolute path, it is taken as relative to
>   the directory containing the referencing configuration file."

  You are right and I have checked code as well, it works in above
way for include directives.
  Now the question I have in mind is that even if we can't
directly use include directive to enable/disable Alter
  System and reading of auto file, how would a new GUC
enable_alter_system can solve all the things.
  It can allow/disallow Alter System command, but how about
reading of auto file?
  If we directly read auto file without considering
enable_alter_system, it can lead to chaos due to some un-safe
  values, on the other side if we want to consider
enable_alter_system before reading file, it can complicate the
  ProcessConfigFile() code.

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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])

[Amit Kapila]

On Wed, Aug 21, 2013 at 7:29 AM, Bruce Momjian  wrote:
> On Tue, Aug 20, 2013 at 08:38:52AM +0530, Amit Kapila wrote:
>> On Tue, Aug 20, 2013 at 8:27 AM, Stephen Frost  wrote:
>> > * Amit Kapila (amit.kapil...@gmail.com) wrote:
>> >> If both are okay, then I would like to go with second option (include
>> >> file mechanism).
>> >> I think by default, we should allow auto file to be included and if
>> >> user faces any problem or otherwise,
>> >> then he can decide to disable it.
>> >
>> > If it's enabled by default, then we need to ship an 'auto' file which is
>> > empty by default...
>>
>> initdb will create the empty auto file. If we don't enable by default,
>> then if user uses
>> ALTER SYSTEM and do sighup/restart, changed config parameter values
>> will not come into affect
>> until he manually enables it by removing '#' from '#include'.
>
> Just a heads-up, but a lot of C language folks are going to look at:
>
> #include abc
>
> and think that is enabled.

True, but generally for conf and script file, symbol '#' is treated as
commented portion of content.


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] Backup throttling

[PostgreSQL - Hans-Jürgen Schönig]

On Aug 19, 2013, at 9:11 PM, Andres Freund wrote:

> On 2013-08-19 20:15:51 +0200, Boszormenyi Zoltan wrote:
>> 2013-08-19 19:20 keltezéssel, Andres Freund írta:
>>> Hi,
>>> 
>>> On 2013-07-24 09:20:52 +0200, Antonin Houska wrote:
 Hello,
 the purpose of this patch is to limit impact of pg_backup on running 
 server.
 Feedback is appreciated.
>>> Based on a quick look it seems like you're throttling on the receiving
>>> side. Is that a good idea? Especially over longer latency links, TCP
>>> buffering will reduce the effect on the sender side considerably.
> 
>> Throttling on the sender side requires extending the syntax of
>> BASE_BACKUP and maybe START_REPLICATION so both can be
>> throttled but throttling is still initiated by the receiver side.
> 
> Seems fine to me. Under the premise that the idea is decided to be
> worthwile to be integrated. Which I am not yet convinced of.

i think there is a lot of value for this one. the scenario we had a couple of 
times is pretty simple:
just assume a weak server - maybe just one disk or two - and a slave.
master and slave are connected via a 1 GB network.
pg_basebackup will fetch data full speed basically putting those lonely disks 
out of business.
we actually had a case where a client asked if "PostgreSQL is locked during 
base backup". of 
course it was just disk wait caused by a full speed pg_basebackup.

regarding the client side implementation: we have chosen this way because it is 
less invasive. 
i cannot see a reason to do this on the server side because we won't have 10 
pg_basebackup-style tools making use of this feature anyway.

of course, if you got 20 disk and a 1 gbit network this is useless - but many 
people don't have that.

regards,

hans-jürgen schönig


--
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt, Austria
Web: http://www.postgresql-support.de



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