Re: [HACKERS] merging some features from plpgsql2 project

[Pavel Stehule]

>
>> Related to that, I suspect we could add better support to existing
>> commands for at least some of these things. For example, SELECT ... INTO
>> NOMULTI (instead of STRICT) to indicate that multiple rows are an error but
>> missing data is
>
>
> Another flag into NOMULTI can be solution too.
>
> The new syntax ":=" has some advantages:
>
> 1. it robust against type - it is harder to do unwanted swap of variables,
> and this mistake is very clear
>

should be "against typo", sorry

Pavel


> 2. the syntax ensure equality of target variables and source expressions.
>
>>
>>
>

Re: [HACKERS] merging some features from plpgsql2 project

[Pavel Stehule]

2017-01-08 3:27 GMT+01:00 Jim Nasby :

> On 1/5/17 11:36 AM, Merlin Moncure wrote:
>
>> The C language really should be considered the gold standard here.
>> Changes did have to be made, like getting rid of the notoriously
>> broken and insecure gets(), but they were made very, very slowly and
>> unobtrusively.
>>
>
> For those not familiar... how did they accomplish that?
>
> I'm certainly fine with changes being made very slowly. We carried the
> missing_From GUC around for like a decade before ditching it. We have other
> GUCs that have defaulted to not allowing silly behavior for a long time as
> well. We might well need to leave the default for compatibility GUCs set to
> current behavior for several more releases, to allow people to start
> specifying which behavior they want.
>
> I agree with Robert that there needs to be consensus that a change needs
> to be made, but frankly I think 50% of this thread was people disagreeing
> with *ANY* change that would be incompatible. IMHO that's a ridiculous
> position that does not match expectations outside of plpgsql. That kind of
> expectation means we have absolutely no way of fixing past mistakes.


> Certainly, there also needs to be agreement on what the new behavior
> should be, but again, what I observed was an adamant insistence that
> absolutely no break would be permitted.


> As for using GUCs for these changes and that impact on extensions, I don't
> see why that won't work for what we're discussing here. In a worst-case
> scenario, extension authors would need to specify what behavior they wanted
> in their extensions instead of blindly accepting the default, by making
> sure those options were set for each function they defined. While it would
> certainly be nice to avoid that extra work, all the necessary
> infrastructure to handle that is already in place. And if we wanted to
> avoid that hassle, we could allow custom GUC settings on extensions, like
> we currently do for roles and databases.


The discussion related to plpgsql2, future development of plpgsql has more
levels, topics

1. incompatible changes - INTO, INTO STRICT, FOUND - there is a agreement
so current behave is not ideal for all cases, but there is not a agreement
so it broken and should be "fixed" - GUC doesn't helps here.

2. new features - the question was "how much we would to move PL/pgSQL from
verbose Ada language to different place - convention against configuration
principle", and what (if) conventions should be default.  GUC can partially
helps.

I still hope so there is some space for finding a partial agreement - and
we can do some evolution steps forward.

I would not to use GUC like "We cannot to find a agreement, so we use GUC
and somebody will use this feature, some one not" - it is not way how to do
things better long time.

Jim, Marko, Joel - is there a place, features where we can find a partial
agreement? If it is, then we can move our view there.

Regards

Pavel


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

Re: [HACKERS] merging some features from plpgsql2 project

[Pavel Stehule]

>
>
>>
>> BTW, I do wish you could change the label of the scope that arguments
>> went into, so that you could use that label to refer to function
>> parameters. If we allowed that it'd perhaps be the best of both worlds:
>> you'd be guaranteed access to all auto variables and parameters, and that
>> access wouldn't need to be tied to the function name (which can be both
>> painful and error prone).
>
>
> We can talk about compiler directive.
>
> PRAGMA auto_variables_label() -- require function scope only
>

If we know a list of all auto variables, then it can be on function or
block level - it can create aliases.

Regards

Pavel


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

Re: [HACKERS] Adding type info etc for inheritance errmsg: "child table is missing column ..."

[Ryan Murphy]

> AFAICS, what Ryan is after would be better served by a separate tool that
> would produce a "diff" of the two tables' schemas.

Related to the other idea of seeing the problems that exist in all the
> columns (instead of one column at a time), I think it'd be reasonable to
> have a SRF that spit out everything you'd need to fix to allow inheritance
> to be added. A schema diff won't know what specifically has to match, but
> our code does.
>
>
Sure, I think that's totally true that I could just make a
Set-Returning-Function (that's what SRF stands for right?) that would
accomplish this.  I'll try that path instead for now.

Thanks guys!
Ryan

Re: [HACKERS] merging some features from plpgsql2 project

[Pavel Stehule]

2017-01-08 3:31 GMT+01:00 Jim Nasby :

> On 1/7/17 5:39 AM, Pavel Stehule wrote:
>
>>
>> I checked current implementation of FOUND variable. If we introduce new
>> auto variable ROW_COUNT - exactly like FOUND, then it doesn't introduce
>> any compatibility break.
>>
>
> Except it would break every piece of code that had a row_count variable,
> though I guess you could see which scoping level the variable had been
> defined in.
>
> I think the right solution in this case is to replace GET DIAGNOSTICs with
> something easier to use, but I'm not sure what that would be.
>

I invite any ideas?

Regards

Pavel


>
> I think this is another example of where not using some kind of character
> to distinguish variables screws us. :/


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

Re: [HACKERS] merging some features from plpgsql2 project

[Pavel Stehule]

2017-01-08 3:39 GMT+01:00 Jim Nasby :

> On 1/7/17 2:06 AM, Pavel Stehule wrote:
>
>>
>> SELECT t1 := c1, t2 := c2, ...
>>
>> - it can be PostgreSQL specific syntax - full control over design
>> - maximally robust against typo
>> - long syntax, but for short syntax can be used SELECT c1,c2,c3, .. INTO
>> STRICT recvar
>>
>
> I don't think overloading a SQL command is a good idea. We'd be in trouble
> if ANSI ever introduced :=. I think that could also conflict with existing
> operators.


The ":=" operator is used ANSI/SQL already for named arguments. Isn't
probable so ANSI uses it in different context.

This is not overloading of SQL command - it is like annotations. It is
smart idea, so I was not surprised if ANSI/SQL reuses it. There is not any
possible construct, that is robust against typo - because assignment is
very verbose and natural.

ANSI - SQL/PSM uses two methods

1. multiassignment

  SET (a,b,c) = (SELECT a, b, c ...)

2. auto variables in dedicated new scope

  FOR scope_label IN SELECT a, b, c
  DO
-- you can use variables a, b, c
-- you can use qualified variables scope_label.a, scope_label.b, ..
  END FOR

This method is not possible in PL/pgSQL  - but a work with record type is
similar


>
>
> - what should be no_data_found behave?
>>
>
> Considering where we're at today, I don't think there should be a default
> behavior; make the user specify somehow whether missing data is allowed or
> not.
>
> I have nothing about a cost of "new syntax" implementation - but for  me
>> - it looks like good solution for us - it can be win/win solution. It
>> breaks nothing - it introduce nice to have typo robust syntax.
>>
>
> Related to that, I suspect we could add better support to existing
> commands for at least some of these things. For example, SELECT ... INTO
> NOMULTI (instead of STRICT) to indicate that multiple rows are an error but
> missing data is


Another flag into NOMULTI can be solution too.

The new syntax ":=" has some advantages:

1. it robust against type - it is harder to do unwanted swap of variables,
and this mistake is very clear
2. the syntax ensure equality of target variables and source expressions.

I see valuable benefit of this syntax

Regards

Pavel



> OK.
>
> --
> Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
> Experts in Analytics, Data Architecture and PostgreSQL
> Data in Trouble? Get it in Treble! http://BlueTreble.com
> 855-TREBLE2 (855-873-2532)
>

Re: [HACKERS] Add support for SRF and returning composites to pl/tcl

[Tom Lane]

Jim Nasby  writes:
> On 1/6/17 2:45 PM, Tom Lane wrote:
>> While I was checking the patch to verify that it didn't change any
>> behavior, I noticed that it did, and there's a pre-existing bug here:
>> pltcl_build_tuple_result is applying utf_e2u to the Tcl_GetString results,
>> but surely that should be utf_u2e instead.  Fortunately we haven't shipped
>> this code yet.

> Ugh.

> For this patch lets just fix that (see attached).

I already fixed it in HEAD.

> Moving forward, I think it'd be good to improve this...

Yeah, it's pretty ugly, but I'm not sure what would be a better design.

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] merging some features from plpgsql2 project

[Pavel Stehule]

2017-01-08 4:11 GMT+01:00 Jim Nasby :

> On 1/7/17 8:53 PM, Tom Lane wrote:
>
>> If FOUND were declared at an outer scoping level such that any
>> user-created declaration overrode the name, then we could do likewise
>> for other auto variables and not fear compatibility breaks.
>>
>> Currently, though, we don't seem to be quite there: it looks like
>> FOUND is an outer variable with respect to DECLARE blocks, but it's
>> more closely nested than parameter names.
>>
>
> Sorry, I'm not following... you can override a parameter name the same way
> and get the same behavior, no?
>

It is declared before any custom identifier. If you override it, then you
are working with own variable - not with auto variable.


>
> BTW, I do wish you could change the label of the scope that arguments went
> into, so that you could use that label to refer to function parameters. If
> we allowed that it'd perhaps be the best of both worlds: you'd be
> guaranteed access to all auto variables and parameters, and that access
> wouldn't need to be tied to the function name (which can be both painful
> and error prone).


We can talk about compiler directive.

PRAGMA auto_variables_label() -- require function scope only

BEGIN
  IF .FOUND THEN

Regards

Pavel


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

Re: [HACKERS] merging some features from plpgsql2 project

[Pavel Stehule]

2017-01-08 3:53 GMT+01:00 Tom Lane :

> Jim Nasby  writes:
> > On 1/7/17 5:39 AM, Pavel Stehule wrote:
> >> I checked current implementation of FOUND variable. If we introduce new
> >> auto variable ROW_COUNT - exactly like FOUND, then it doesn't introduce
> >> any compatibility break.
>
> > Except it would break every piece of code that had a row_count variable,
> > though I guess you could see which scoping level the variable had been
> > defined in.
>
> If FOUND were declared at an outer scoping level such that any
> user-created declaration overrode the name, then we could do likewise
> for other auto variables and not fear compatibility breaks.
>
> Currently, though, we don't seem to be quite there: it looks like
> FOUND is an outer variable with respect to DECLARE blocks, but it's
> more closely nested than parameter names.  Compare:
>
> regression=# create function foo1(bool) returns bool as
> 'declare found bool := $1; begin return found; end' language plpgsql;
> CREATE FUNCTION
> regression=# select foo1(true);
>  foo1
> --
>  t
> (1 row)
>
> regression=# create function foo2(found bool) returns bool as
> regression-# 'begin return found; end' language plpgsql;
> CREATE FUNCTION
> regression=# select foo2(true);
>  foo2
> --
>  f
> (1 row)
>
> Not sure if changing this would be a good thing or not --- was
> there reasoning behind this behavior, or was it just accidental?
>

There are two related features in plpgsql2 project:

1. dynamic SQL sets FOUND variable
2. direct access to processed rows info via variable ROW_COUNT

@1 is incompatible change, @2 is good enough - so we should not to change
FOUND, but we can propagate ROW_COUNT instead.

Regards

Pavel



>
> regards, tom lane
>

Re: [HACKERS] Add support for SRF and returning composites to pl/tcl

[Jim Nasby]

On 1/6/17 2:45 PM, Tom Lane wrote:

The only thing that seems significant is that we'd change the SQLSTATE
for the "odd number of list items" error: pltcl_trigger_handler has

(errcode(ERRCODE_E_R_I_E_TRIGGER_PROTOCOL_VIOLATED),
 errmsg("trigger's return list must have even number of elements")));

and that would become

(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("column name/value list must have even number of elements")));

But that's probably fine; it's hard to believe anyone is depending on this
particular case --- and I think the argument that this is legitimately
a TRIGGER_PROTOCOL_VIOLATED case is a bit thin anyway.


Yeah, I forgot to mention that, but I did look at both cases and felt 
the same.



While I was checking the patch to verify that it didn't change any
behavior, I noticed that it did, and there's a pre-existing bug here:
pltcl_build_tuple_result is applying utf_e2u to the Tcl_GetString results,
but surely that should be utf_u2e instead.  Fortunately we haven't shipped
this code yet.


Ugh.

For this patch lets just fix that (see attached). Moving forward, I 
think it'd be good to improve this...


There's only 2 cases of Tcl_GetString that don't then call utf_u2e (or, 
amusingly, UTF_U2E); perhaps we should just get rid of all direct use of 
TclGetString and replace it with something like pltcl_get_utf() and 
pltcl_get_ascii()?


Oh, hell, now I see that there's an actual difference between utf_* and 
UTF_*. And AFAICT, those macros don't completely solve things either; if 
you make multiple calls you're still leaking memory but wouldn't know 
it. Granted, existing calls seem to all be properly wrapped, but that 
seems pretty error prone.


Looking at plpython, it seems that it doesn't go through any of this 
trouble: if you're using python 3 it just treats everything as if it 
needs encoding, and the conversion routines handle freeing things. 
AFAICT that's true for identifiers as well.


So I'm thinking that we should just forbid the use of direct Tcl string 
functions and pass everything though our own functions that will handle 
UTF8. There are a bunch of places where we're handing static C strings 
(that are clearly plain ASCII) to TCL, so we should probably continue to 
support that. But in the other direction I don't think it's worth it.


TCL does have a separate string append operation as well, so we'll need 
to either provide that or do all the appending using our string 
functions and then pass that along.

--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)
diff --git a/src/pl/tcl/pltcl.c b/src/pl/tcl/pltcl.c
index 5cb4ee85e0..0524db6548 100644
--- a/src/pl/tcl/pltcl.c
+++ b/src/pl/tcl/pltcl.c
@@ -3035,7 +3035,7 @@ pltcl_build_tuple_result(Tcl_Interp *interp, Tcl_Obj 
**kvObjv, int kvObjc,
 
for (i = 0; i < kvObjc; i += 2)
{
-   char   *fieldName = utf_e2u(Tcl_GetString(kvObjv[i]));
+   char   *fieldName = utf_u2e(Tcl_GetString(kvObjv[i]));
int attn = 
SPI_fnumber(call_state->ret_tupdesc, fieldName);
 
/*
@@ -3058,7 +3058,7 @@ pltcl_build_tuple_result(Tcl_Interp *interp, Tcl_Obj 
**kvObjv, int kvObjc,
 errmsg("cannot set system attribute 
\"%s\"",
fieldName)));
 
-   values[attn - 1] = utf_e2u(Tcl_GetString(kvObjv[i + 1]));
+   values[attn - 1] = utf_u2e(Tcl_GetString(kvObjv[i + 1]));
}
 
return BuildTupleFromCStrings(call_state->attinmeta, values);

-- 
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] Adding type info etc for inheritance errmsg: "child table is missing column ..."

[Ryan Murphy]

> Perhaps the ERROR message should remain as is, and a DETAIL or HINT line
> could be emitted with the entire column definition (or close to it)?
>
>
I'm open to this option as well.  The only parts I really care about are
the type and the NOT NULL, since those are the ones that will give you an
error if it doesn't line up with the parent.  Putting it in a DETAIL or
HINT is fine with me.

Re: [HACKERS] [PATCH] Rename pg_switch_xlog to pg_switch_wal

[Jim Nasby]

On 1/6/17 7:21 PM, Bruce Momjian wrote:

I don't think anyone is arguing that these API breakages are cost-free,
but I think long-term, the costs are minor compared to the clean API we
provide to users.


Except in this case we can provide a clean new API without gratuitously 
breaking the old one.


I absolutely do agree that we have to have a way of making incompatible 
changes from time to time (as I've been arguing over and over in the 
plpgsql2 thread). I also think a one-size-fits-all approach to when a 
break is allowed would be good. That said, the change to 
pg_stat_activity caused a lot of needless user pain, and this one will 
as well.


What makes this even more frustrating (to me at least) is that the 
attitude that's been demonstrated around plpgsql is that there can 
absolutely NEVER be a compatibility break of any kind.


So part of the project thinks it's perfectly OK to just break things 
without any warning or support, while another part of the project 
adamantly refuses any kind of a break at all, even what's breaking has 
never officially been supported.


I don't think that dichotomy is good for the community or for our users.
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)


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

Re: [HACKERS] pg_background contrib module proposal

[Jim Nasby]

On 12/22/16 4:30 PM, Robert Haas wrote:

On Thu, Dec 22, 2016 at 4:41 PM, Jim Nasby  wrote:

On 12/22/16 4:20 AM, amul sul wrote:

• pg_background_detach : This API takes the process id and detach the
background process. Stored worker's session is not dropped until this
called.


When I hear "detach" I think that whatever I'm detaching from is going to
stick around, which I don't think is the case here, right? I'd suggest
pg_background_close() instead.


Uh, I think it is.  At least in the original version of this patch,
pg_background_detach() leaves the spawned process running but says
that you don't care to read any results it may generate.



Oh, hmm. So I guess if you do that when the background process is idle 
it's the same as a close?


I think we need some way to safeguard against accidental forkbombs for 
cases where users aren't intending to leave something running in the 
background. There's other reasons to use this besides spawning long 
running processes, and I'd certainly want to be able to ensure the 
calling function wasn't accidentally leaving things running that it 
didn't mean to. (Maybe the patch already does this...)

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


--
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] merging some features from plpgsql2 project

[Jim Nasby]

On 1/7/17 8:53 PM, Tom Lane wrote:

If FOUND were declared at an outer scoping level such that any
user-created declaration overrode the name, then we could do likewise
for other auto variables and not fear compatibility breaks.

Currently, though, we don't seem to be quite there: it looks like
FOUND is an outer variable with respect to DECLARE blocks, but it's
more closely nested than parameter names.


Sorry, I'm not following... you can override a parameter name the same 
way and get the same behavior, no?


BTW, I do wish you could change the label of the scope that arguments 
went into, so that you could use that label to refer to function 
parameters. If we allowed that it'd perhaps be the best of both worlds: 
you'd be guaranteed access to all auto variables and parameters, and 
that access wouldn't need to be tied to the function name (which can be 
both painful and error prone).

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


--
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] ICU integration

[Peter Geoghegan]

On Tue, Dec 27, 2016 at 6:50 PM, Peter Eisentraut
 wrote:
> I don't have much experience with the abbreviated key stuff.  I have
> filled in what I think should work, but it needs detailed review.

Thanks.

It occurs to me that the comparison caching stuff added by commit
0e57b4d8b needs to be considered here, too. When we had to copy the
string to a temp buffer anyway, in order to add the terminating NUL
byte expected by strcoll(), there was an opportunity to do caching of
comparisons at little additional cost. However, since ICU offers an
interface that you're using that doesn't require any NUL byte, there
is a new trade-off to be considered -- swallow the cost of copying
into our own temp buffer solely for the benefit of comparison caching,
or don't do comparison caching. (Note that glibc had a similar
comparison caching optimization itself at one point, built right into
strcoll(), but it was subsequently disabled.)

-- 
Peter Geoghegan


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

Re: [HACKERS] [WIP]Vertical Clustered Index (columnar store extension)

[Jim Nasby]

On 12/29/16 9:55 PM, Haribabu Kommi wrote:

The tuples which don't have multiple copies or frozen data will be moved
from WOS to ROS periodically by the background worker process or autovauum
process. Every column data is stored separately in it's relation file. There
is no transaction information is present in ROS. The data in ROS can be
referred with tuple ID.


Would updates be handled via the delete mechanism you described then?


In this approach, the column data is present in both heap and columnar
storage.


ISTM one of the biggest reasons to prefer a column store over heap is to 
ditch the 24 byte overhead, so I'm not sure how much of a win this is.


Another complication is that one of the big advantages of a CSTORE is 
allowing analysis to be done efficiently on a column-by-column (as 
opposed to row-by-row) basis. Does your patch by chance provide that?


Generally speaking, I do think the idea of adding support for this as an 
"index" is a really good starting point, since that part of the system 
is pluggable. It might be better to target getting only what needs to be 
in core into core to begin with, allowing the other code to remain an 
extension for now. I think there's a lot of things that will be 
discovered as we start moving into column stores, and it'd be very 
unfortunate to accidentally paint the core code into a corner somewhere.


As a side note, it's possible to get a lot of the benefits of a column 
store by using arrays. I've done some experiments with that and got an 
80-90% space reduction, and most queries saw improved performance as 
well (there were a few cases that weren't better). The biggest advantage 
to this approach is people could start using it today, on any recent 
version of Postgres. That would be a great way to gain knowledge on what 
users would want to see in a column store, something else I suspect we 
need. It would also be far less code than what you or Alvaro are 
proposing. When it comes to large changes that don't have crystal-clear 
requirements, I think that's really important.

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


--
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] merging some features from plpgsql2 project

[Tom Lane]

Jim Nasby  writes:
> On 1/7/17 5:39 AM, Pavel Stehule wrote:
>> I checked current implementation of FOUND variable. If we introduce new
>> auto variable ROW_COUNT - exactly like FOUND, then it doesn't introduce
>> any compatibility break.

> Except it would break every piece of code that had a row_count variable, 
> though I guess you could see which scoping level the variable had been 
> defined in.

If FOUND were declared at an outer scoping level such that any
user-created declaration overrode the name, then we could do likewise
for other auto variables and not fear compatibility breaks.

Currently, though, we don't seem to be quite there: it looks like
FOUND is an outer variable with respect to DECLARE blocks, but it's
more closely nested than parameter names.  Compare:

regression=# create function foo1(bool) returns bool as
'declare found bool := $1; begin return found; end' language plpgsql;
CREATE FUNCTION
regression=# select foo1(true);
 foo1 
--
 t
(1 row)

regression=# create function foo2(found bool) returns bool as
regression-# 'begin return found; end' language plpgsql;
CREATE FUNCTION
regression=# select foo2(true);
 foo2 
--
 f
(1 row)

Not sure if changing this would be a good thing or not --- was
there reasoning behind this behavior, or was it just accidental?

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] merging some features from plpgsql2 project

[Jim Nasby]

On 1/7/17 2:06 AM, Pavel Stehule wrote:


SELECT t1 := c1, t2 := c2, ...

- it can be PostgreSQL specific syntax - full control over design
- maximally robust against typo
- long syntax, but for short syntax can be used SELECT c1,c2,c3, .. INTO
STRICT recvar


I don't think overloading a SQL command is a good idea. We'd be in 
trouble if ANSI ever introduced :=. I think that could also conflict 
with existing operators.



- what should be no_data_found behave?


Considering where we're at today, I don't think there should be a 
default behavior; make the user specify somehow whether missing data is 
allowed or not.



I have nothing about a cost of "new syntax" implementation - but for  me
- it looks like good solution for us - it can be win/win solution. It
breaks nothing - it introduce nice to have typo robust syntax.


Related to that, I suspect we could add better support to existing 
commands for at least some of these things. For example, SELECT ... INTO 
NOMULTI (instead of STRICT) to indicate that multiple rows are an error 
but missing data is OK.

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


--
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] merging some features from plpgsql2 project

[Jim Nasby]

On 1/7/17 5:39 AM, Pavel Stehule wrote:


I checked current implementation of FOUND variable. If we introduce new
auto variable ROW_COUNT - exactly like FOUND, then it doesn't introduce
any compatibility break.


Except it would break every piece of code that had a row_count variable, 
though I guess you could see which scoping level the variable had been 
defined in.


I think the right solution in this case is to replace GET DIAGNOSTICs 
with something easier to use, but I'm not sure what that would be.


I think this is another example of where not using some kind of 
character to distinguish variables screws us. :/

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


--
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] merging some features from plpgsql2 project

[Jim Nasby]

On 1/5/17 11:36 AM, Merlin Moncure wrote:

The C language really should be considered the gold standard here.
Changes did have to be made, like getting rid of the notoriously
broken and insecure gets(), but they were made very, very slowly and
unobtrusively.


For those not familiar... how did they accomplish that?

I'm certainly fine with changes being made very slowly. We carried the 
missing_From GUC around for like a decade before ditching it. We have 
other GUCs that have defaulted to not allowing silly behavior for a long 
time as well. We might well need to leave the default for compatibility 
GUCs set to current behavior for several more releases, to allow people 
to start specifying which behavior they want.


I agree with Robert that there needs to be consensus that a change needs 
to be made, but frankly I think 50% of this thread was people 
disagreeing with *ANY* change that would be incompatible. IMHO that's a 
ridiculous position that does not match expectations outside of plpgsql. 
That kind of expectation means we have absolutely no way of fixing past 
mistakes.


Certainly, there also needs to be agreement on what the new behavior 
should be, but again, what I observed was an adamant insistence that 
absolutely no break would be permitted.


As for using GUCs for these changes and that impact on extensions, I 
don't see why that won't work for what we're discussing here. In a 
worst-case scenario, extension authors would need to specify what 
behavior they wanted in their extensions instead of blindly accepting 
the default, by making sure those options were set for each function 
they defined. While it would certainly be nice to avoid that extra work, 
all the necessary infrastructure to handle that is already in place. And 
if we wanted to avoid that hassle, we could allow custom GUC settings on 
extensions, like we currently do for roles and databases.

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


--
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] Assignment of valid collation for SET operations on queries with UNKNOWN types.

[Tom Lane]

Ashutosh Bapat  writes:
> On Tue, Jan 3, 2017 at 5:57 PM, Rahila Syed  wrote:
>> Are you suggesting extending the patch to include coercing from unknown to
>> text for all possible cases where a column of unknown type is being created?

> I guess, that's what Tom is suggesting.

Yes; I think the point is we should change the semantics we assume for
"SELECT 'foo'", not only for views but across the board.  There are plenty
of non-view-related cases where it doesn't behave well, for example

regression=# select * from
  (select 'foo' from generate_series(1,3)) ss order by 1;
ERROR:  failed to find conversion function from unknown to text

Furthermore, it seems like a seriously bad idea for "SELECT 'foo'" to mean
something different in the context of CREATE VIEW than it means elsewhere.

The trick is to not break cases where we've already hacked things to allow
external influence on the resolved types of SELECT-list items.  AFAICT,
these cases are just INSERT/SELECT and set operations (eg unions):

regression=# create table target (f1 int);
CREATE TABLE
regression=# explain verbose insert into target select '42' from 
generate_series(1,3);
   QUERY PLAN   
 

-
 Insert on public.target  (cost=0.00..10.00 rows=1000 width=4)
   ->  Function Scan on pg_catalog.generate_series  (cost=0.00..10.00 rows=1000 
width=4)
 Output: 42
 Function Call: generate_series(1, 3)
(4 rows)

regression=# explain verbose select '42' union all select 43;
   QUERY PLAN   

 Append  (cost=0.00..0.04 rows=2 width=4)
   ->  Result  (cost=0.00..0.01 rows=1 width=4)
 Output: 42
   ->  Result  (cost=0.00..0.01 rows=1 width=4)
 Output: 43
(5 rows)

In both of the above cases, we're getting an integer constant not a
text or "unknown" constant, because the type information gets imported
from outside the "SELECT".

I spent some time fooling with this today and came up with the attached
patch.  I think this is basically the direction we should go in, but
there are various loose ends:

1. I adjusted a couple of existing regression tests whose results are
affected, but didn't do anything towards adding new tests showing the
desirable results of this change (as per the examples in this thread).

2. I didn't do anything about docs, either, though maybe no change
is needed.  One user-visible change from this is that queries should
never return any "unknown"-type columns to clients anymore.  But I
think that is not documented now, so maybe there's nothing to change.

3. We need to look at whether pg_upgrade is affected.  I think we
might be able to let it just ignore the issue for views, since they'd
get properly updated during the dump and reload anyway.  But if someone
had an actual table (or matview) with an "unknown" column, that would
be a pg_upgrade hazard, because "unknown" doesn't store like "text".

4. If "unknown" were marked as a pseudotype not base type in pg_type
(ie, typtype "p" not "b") then the special case for it in
CheckAttributeType could go away completely.  I'm not really sure
why it's "b" today --- maybe specifically because of this point that
it's currently possible to create tables with unknown columns?  But
I've not looked at what all the implications of changing that might
be, and anyway it could be left for a followon patch.

5. I noticed that the "resolveUnknown" arguments of transformSortClause
and other functions in parse_clause.c could go away: there's really no
reason not to just treat them as "true" always.  But that could be
separate cleanup as well.

Anybody want to hack on those loose ends?

regards, tom lane

diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 72aa0dd..af6ba47 100644
*** a/src/backend/catalog/heap.c
--- b/src/backend/catalog/heap.c
*** CheckAttributeType(const char *attname,
*** 490,507 
  	char		att_typtype = get_typtype(atttypid);
  	Oid			att_typelem;
  
! 	if (atttypid == UNKNOWNOID)
! 	{
! 		/*
! 		 * Warn user, but don't fail, if column to be created has UNKNOWN type
! 		 * (usually as a result of a 'retrieve into' - jolly)
! 		 */
! 		ereport(WARNING,
! (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
!  errmsg("column \"%s\" has type \"unknown\"", attname),
!  errdetail("Proceeding with relation creation anyway.")));
! 	}
! 	else if (att_typtype == TYPTYPE_PSEUDO)
  	{
  		/*
  		 * Refuse any attempt to create a pseudo-type column, except for a
--- 490,497 
  	char		att_typtype = get_typtype(atttypid);
  	Oid			att_typelem;
  
! 	if (atttypid == UNKNOWNOID ||
! 		att_typtype == TYPTYPE_PSEUDO)
  	{
  		/*
  		 * Refuse any attempt to create a pseudo-type column, except for a
diff --git 

[HACKERS] Allow controlling location of tmp_install

[Jim Nasby]

While running a couple of parallel make (install)check-worlds I was 
surprised to see my system load at 5x what I'd think it'd be. Looking at 
CPU % consumption it turns out this was because mds (the OS X indexing 
service) and a few other things were going bonkers. That's because they 
were all trying to process a huge amount of notification events.


I've now moved my normal install locations to somewhere I can exclude 
from mds and what-not, but I don't see any way to do that with 
tmp_install. Is there a trick here I'm missing, or should I change 
Makefile.global.in to check for an environment variable?

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


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

Re: [HACKERS] [PATCH] guc-ify the formerly hard-coded MAX_SEND_SIZE to max_wal_send

[Jim Nasby]

On 1/5/17 12:55 PM, Jonathon Nelson wrote:

Attached please find a patch for PostgreSQL 9.4 which changes the
maximum amount of data that the wal sender will send at any point in
time from the hard-coded value of 128KiB to a user-controllable value up
to 16MiB. It has been primarily tested under 9.4 but there has been some
testing with 9.5.


To make sure this doesn't get lost, please add it to 
https://commitfest.postgresql.org. Please verify the patch will apply 
against current HEAD and pass make check-world.

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


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

Re: [HACKERS] Re: Clarifying "server starting" messaging in pg_ctl start without --wait

[Jim Nasby]

On 1/6/17 12:24 AM, Ryan Murphy wrote:

I don't actually believe this to indicate a problem though - I think perhaps there's a 
problem with this test, or with how I am running it.  The only diff was that when it 
(correctly) complained of a nonexistent database, it referred to my username that I was 
logged in as, instead of the test database name "regress_ecpg_user2".  I don't 
think this has anything to do with the changes to pg_ctl.


Hrm, I'm not able to reproduce that problem. Can you run make 
installworld-check on a checkout of master and see if you get the same 
thing?

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


--
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] Adding type info etc for inheritance errmsg: "child table is missing column ..."

[Jim Nasby]

On 1/7/17 1:16 PM, Ryan Murphy wrote:


No, and TBH I would vote strongly against including that much detail in
this error message anyway.  That info could be indefinitely long,
and it's
not especially relevant to the stated error condition --- for
example, the
presence of a default is *not* relevant to whether the column
matches the
parent.  I'm okay with shoehorning column type into this message,
but not
much more than that.

regards, tom lane


Ok, that makes sense.  How about things like NOT NULL? you get an error
if your column doesn't have that.


Yeah, anything that we're explicitly testing for needs to be mentioned 
in an error message, otherwise users will be very confused if the column 
*is* in the parent but is failing some other test. Perhaps it would be 
better for each test to spit out a different error message making it 
clear what exactly was wrong.


Related to the other idea of seeing the problems that exist in all the 
columns (instead of one column at a time), I think it'd be reasonable to 
have a SRF that spit out everything you'd need to fix to allow 
inheritance to be added. A schema diff won't know what specifically has 
to match, but our code does.

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


--
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] Replication/backup defaults

[Jim Nasby]

On 1/5/17 2:50 PM, Tomas Vondra wrote:

Ultimately, the question is whether the number of people running into
"Hey, I can't take pg_basebackup or setup a standby with the default
config!" is higher or lower than number of people running into "Hey,
CREATE TABLE + COPY is slower now!"


I'm betting it's way higher. Loads of folks use Postgres and never do 
any kind of ETL.



That is not to say there are no other cases benefiting from those
optimizations, but we're talking about the default value - we're not
removing the wal_level=minimal.


This would be a non-issue if we provided example configs for a few 
different workloads. Obviously those would never be optimal either, but 
they *would* show users what settings they should immediately look at 
changing in their environment.

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


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

Re: [HACKERS] pg_stat_activity.waiting_start

[Jim Nasby]

On 1/7/17 12:41 PM, Joel Jacobson wrote:

On Sat, Jan 7, 2017 at 3:25 AM, Greg Stark  wrote:

What users need to know is in aggregate how much of the time the
database is spending working on their queries is going into different
states.


This is a separate feature idea, but I think it's really valuable as well.

Maybe something similar to pg_stat_user_functions?
But instead grouping by wait_event_type, wait_event, and showing
accumulated count and sum of waiting time since last stat reset, just
like the other pg_stat_* views?

Maybe something like this?

\d pg_stat_waiting
 View "pg_catalog.pg_stat_waiting"
   Column|   Type   | Modifiers
-+--+---
 wait_event_type | name |
 wait_event  | name |
 waiting_counter | bigint   |
 waiting_time| double precision |


Yes, I've wanted this many times in the past. If combined with Robert's 
idea of a background process that does the expensive time calls this 
could potentially provide very useful information for even very short 
duration locks.

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


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

Re: [HACKERS] pg_stat_activity.waiting_start

[Jim Nasby]

On 12/28/16 10:26 PM, Robert Haas wrote:

On Wed, Dec 28, 2016 at 1:06 PM, Tom Lane  wrote:

Jim Nasby  writes:

On 12/28/16 11:25 AM, Tom Lane wrote:

The idea of just capturing the wait start for heavyweight locks, and
not other lock types, still seems superior to any of the alternatives
that have been suggested ...



Is some kind of alarm a viable option for the others? If setting the
alarm is cheap,


Setting an alarm is certain to require a gettimeofday and/or a kernel
call.  It is far from cheap.


If one were OK with a really-really-rough value for when the wait
started, you could have a slot for a timestamp in PGPROC that is
cleared by pgstat_report_wait_end() but not set by
pgstat_report_wait_start().  Then you could have a background process
that goes through and sets it for all processes advertising a wait
event every second, or every 10 seconds, or every half second, or
whatever you want.  Of course this doesn't eliminate the overhead but
it pushes it into the background which, if there are idle CPUs, is
almost as good.

I previously proposed something similar to this as a way of profiling
waits and I believe you weren't very supportive of it, but I still
think these kinds of ideas have some legs.  We can neither take the
approach that timestamp overhead is irrelevant nor the position that
timestamps are expensive so let's not have any.  Some third way has to
be found.


I like that idea as a compromise. I've spent a lot of time looking at 
systems with no process > 90% CPU and IO < 90% yet individual backends 
being slow and wished I had some idea of what the backend was doing that 
wasn't being accounted for.


BTW, instead of a flag I'd consider using a counter. That would allow 
the background process to notice if backends were going into waits that 
ultimately got resolved in between scans of PGPROC. That's obviously not 
useful for pg_stat_activity, but it would be meaningful for anything 
that was trying to accumulate wait time stats.

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


--
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] increasing the default WAL segment size

[Jim Nasby]

On 1/5/17 5:38 AM, Beena Emerson wrote:

On Tue, Jan 3, 2017 at 2:53 AM, Jim Nasby > wrote:
General comments:
There was some discussion about the impact of this on small
installs, particularly around min_wal_size. The concern was that

...

The patch maintains the current XLOG_SEG_SIZE of 16MB as the default.
Only the capability to change its value has been moved for configure to
initdb.


Ah, I missed that. Thanks for clarifying.


It's not clear from the thread that there is consensus that this
feature is desired. In particular, the performance aspects of
changing segment size from a C constant to a variable are in
question. Someone with access to large hardware should test that.
Andres[1] and Robert[2] did suggest that the option could be changed
to a bitshift, which IMHO would also solve some sanity-checking issues.


Are you going to change to a bitshift in the next patch?


+* initdb passes the WAL segment size in an environment
variable. We don't
+* bother doing any sanity checking, we already check in
initdb that the
+* user gives a sane value.

That doesn't seem like a good idea to me. If anything, the backend
should sanity-check and initdb just rely on that. Perhaps this is
how other initdb options work, but it still seems bogus. In
particular, verifying the size is a power of 2 seems important, as
failing that would probably be ReallyBad(tm).

The patch also blindly trusts the value read from the control file;
I'm not sure if that's standard procedure or not, but ISTM it'd be
worth sanity-checking that as well.


There is a CRC check to detect error in the file. I think all the
ControlFile values are used directly and not re-verified.


Sounds good. I do still think the variable from initdb should be 
sanity-checked.



The patch leaves the base GUC units for min_wal_size and
max_wal_size as the # of segments. I'm not sure if that's a great idea.


I think we can leave it as is. This is used in
CalculateCheckpontSegments and in XLOGfileslop to calculate the segment
numbers.


My concern here is that we just changed from segments to KB for all the 
checkpoint settings, and this is introducing segments back in, but ...



I agree pretty_print_kb would have been a better for this function.
However, I have realised that using the show hook and this function is
not suitable and have found a better way of handling the removal of
GUC_UNIT_XSEGS which no longer needs this function : using the
GUC_UNIT_KB, convert the value in bytes to wal segment count instead in
the assign hook. The next version of patch will use this.


... it sounds like you're going back to exposing KB to users, and that's 
all that really matters.



IMHO it'd be better to use the n & (n-1) check detailed at [3].


See my other email about that.
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)


--
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] increasing the default WAL segment size

[Jim Nasby]

On 1/4/17 10:03 PM, David Rowley wrote:

I recall taht pow(x, 2) and x * x result usually in the same assembly
code, but pow() can never be more optimal than a simple
multiplication. So I'd think that it is wiser to avoid it in this code
path. Documentation is missing for the new replication command
SHOW_WAL_SEG. Actually, why not just having an equivalent of the SQL
command and be able to query parameter values?

This would probably be nicer written using a bitwise trick to ensure
that no lesser significant bits are set. If it's a power of 2, then
subtracting 1 should have all the lesser significant bits as 1, so
binary ANDing to that should be 0. i.e no common bits.

Something like:

/* ensure segment size is a power of 2 */
if ((wal_segment_size & (wal_segment_size - 1)) != 0)
{
   fprintf(stderr, _("%s: WAL segment size must be in the power of
2\n"), progname);
   exit(1);
}

There's a similar trick in bitmapset.c for RIGHTMOST_ONE, so looks
like we already have assumptions about two's complement arithmetic


Well, now that there's 3 places that need to do almost the same thing, I 
think it'd be best to just centralize this somewhere. I realize that's 
not going to save any significant amount of code, but it would make it 
crystal clear what's going on (assuming the excellent comment above 
RIGHTMOST_ONE was kept).

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


--
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] Increase pltcl test coverage

[Jim Nasby]

On 1/6/17 2:17 PM, Tom Lane wrote:

Jim Nasby  writes:

On 10/31/16 3:24 PM, Jim Nasby wrote:

This patch increases test coverage for pltcl, from 70% to 83%. Aside
from that, the work on this uncovered 2 new bugs (the trigger return one
I just submitted, as well as a bug in the SRF/composite patch).



Rebased patch attached. Test coverage is now at 85% (by line count).


This is in a format that neither patch(1) nor "git apply" recognize.
Please resubmit in a more usual format, diff -c or diff -u perhaps.


Odd, dunno what happened there. New patch attached.

BTW, this also includes some case changes back to lowercase. My muscle 
memory would prefer to go the other direction but I figured consistency 
was worth something. Feel free not to include that bit if you want.

--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)
diff --git a/src/pl/tcl/expected/pltcl_queries.out 
b/src/pl/tcl/expected/pltcl_queries.out
index 3a9fef3447..4794e88a5b 100644
--- a/src/pl/tcl/expected/pltcl_queries.out
+++ b/src/pl/tcl/expected/pltcl_queries.out
@@ -1,3 +1,7 @@
+BEGIN;
+SET LOCAL client_min_messages = WARNING;
+CREATE EXTENSION IF NOT EXISTS plpgsql;
+COMMIT;
 -- suppress CONTEXT so that function OIDs aren't in output
 \set VERBOSITY terse
 insert into T_pkey1 values (1, 'key1-1', 'test key');
@@ -185,12 +189,23 @@ select * from T_pkey2 order by key1 using @<, key2 
collate "C";
 
 -- show dump of trigger data
 insert into trigger_test values(1,'insert');
-NOTICE:  NEW: {i: 1, v: insert}
+NOTICE:  NEW: {}
+NOTICE:  OLD: {}
+NOTICE:  TG_level: STATEMENT
+NOTICE:  TG_name: statement_trigger
+NOTICE:  TG_op: INSERT
+NOTICE:  TG_relatts: {{} i v {} test_skip test_return_null test_argisnull}
+NOTICE:  TG_relid: bogus:12345
+NOTICE:  TG_table_name: trigger_test
+NOTICE:  TG_table_schema: public
+NOTICE:  TG_when: BEFORE
+NOTICE:  args: {42 {statement trigger}}
+NOTICE:  NEW: {i: 1, test_argisnull: f, test_return_null: f, test_skip: f, v: 
insert}
 NOTICE:  OLD: {}
 NOTICE:  TG_level: ROW
 NOTICE:  TG_name: show_trigger_data_trig
 NOTICE:  TG_op: INSERT
-NOTICE:  TG_relatts: {{} i v}
+NOTICE:  TG_relatts: {{} i v {} test_skip test_return_null test_argisnull}
 NOTICE:  TG_relid: bogus:12345
 NOTICE:  TG_table_name: trigger_test
 NOTICE:  TG_table_schema: public
@@ -232,13 +247,37 @@ NOTICE:  TG_table_name: trigger_test_view
 NOTICE:  TG_table_schema: public
 NOTICE:  TG_when: {INSTEAD OF}
 NOTICE:  args: {24 {skidoo view}}
+update trigger_test set v = 'update', test_skip=true where i = 1;
+NOTICE:  NEW: {}
+NOTICE:  OLD: {}
+NOTICE:  TG_level: STATEMENT
+NOTICE:  TG_name: statement_trigger
+NOTICE:  TG_op: UPDATE
+NOTICE:  TG_relatts: {{} i v {} test_skip test_return_null test_argisnull}
+NOTICE:  TG_relid: bogus:12345
+NOTICE:  TG_table_name: trigger_test
+NOTICE:  TG_table_schema: public
+NOTICE:  TG_when: BEFORE
+NOTICE:  args: {42 {statement trigger}}
+NOTICE:  SKIPPING OPERATION UPDATE
 update trigger_test set v = 'update' where i = 1;
-NOTICE:  NEW: {i: 1, v: update}
-NOTICE:  OLD: {i: 1, v: insert}
+NOTICE:  NEW: {}
+NOTICE:  OLD: {}
+NOTICE:  TG_level: STATEMENT
+NOTICE:  TG_name: statement_trigger
+NOTICE:  TG_op: UPDATE
+NOTICE:  TG_relatts: {{} i v {} test_skip test_return_null test_argisnull}
+NOTICE:  TG_relid: bogus:12345
+NOTICE:  TG_table_name: trigger_test
+NOTICE:  TG_table_schema: public
+NOTICE:  TG_when: BEFORE
+NOTICE:  args: {42 {statement trigger}}
+NOTICE:  NEW: {i: 1, test_argisnull: f, test_return_null: f, test_skip: f, v: 
update}
+NOTICE:  OLD: {i: 1, test_argisnull: f, test_return_null: f, test_skip: f, v: 
insert}
 NOTICE:  TG_level: ROW
 NOTICE:  TG_name: show_trigger_data_trig
 NOTICE:  TG_op: UPDATE
-NOTICE:  TG_relatts: {{} i v}
+NOTICE:  TG_relatts: {{} i v {} test_skip test_return_null test_argisnull}
 NOTICE:  TG_relid: bogus:12345
 NOTICE:  TG_table_name: trigger_test
 NOTICE:  TG_table_schema: public
@@ -246,16 +285,39 @@ NOTICE:  TG_when: BEFORE
 NOTICE:  args: {23 skidoo}
 delete from trigger_test;
 NOTICE:  NEW: {}
-NOTICE:  OLD: {i: 1, v: update}
+NOTICE:  OLD: {}
+NOTICE:  TG_level: STATEMENT
+NOTICE:  TG_name: statement_trigger
+NOTICE:  TG_op: DELETE
+NOTICE:  TG_relatts: {{} i v {} test_skip test_return_null test_argisnull}
+NOTICE:  TG_relid: bogus:12345
+NOTICE:  TG_table_name: trigger_test
+NOTICE:  TG_table_schema: public
+NOTICE:  TG_when: BEFORE
+NOTICE:  args: {42 {statement trigger}}
+NOTICE:  NEW: {}
+NOTICE:  OLD: {i: 1, test_argisnull: f, test_return_null: f, test_skip: f, v: 
update}
 NOTICE:  TG_level: ROW
 NOTICE:  TG_name: show_trigger_data_trig
 NOTICE:  TG_op: DELETE
-NOTICE:  TG_relatts: {{} i v}
+NOTICE:  TG_relatts: {{} i v {} test_skip test_return_null test_argisnull}
 NOTICE:  TG_relid: bogus:12345
 NOTICE:  TG_table_name: trigger_test
 NOTICE:  TG_table_schema: public
 NOTICE:  

Re: [HACKERS] proposal: session server side variables (fwd)

[Bruce Momjian]

On Thu, Jan  5, 2017 at 11:45:24AM +0100, Fabien COELHO wrote:
> 
>   I must tweak my mail client configuration...>
> 
> >>>Good. So we seem to agree that GUCS are transactional?
> >
> >I'm surprised, I never knew this.
> 
> I must admit that it was also a (good) surprise for me.
> 
> The documentation says it:
> 
> """
> If SET (or equivalently SET SESSION) is issued within a transaction that is
> later aborted, the effects of the SET command disappear when the transaction
> is rolled back. Once the surrounding transaction is committed, the effects
> will persist until the end of the session, unless overridden by another SET.
> """
> 
> But I have not found anything clear about user-defined parameters.

Uh, I think it is a missing feature, i.e.:

https://wiki.postgresql.org/wiki/Todo#Administration
Have custom variables be transaction-safe 

https://www.postgresql.org/message-id/4b577e9f.8000...@dunslane.net

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

+ As you are, so once was I.  As I am, so you will be. +
+  Ancient Roman grave inscription +


-- 
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] Adding type info etc for inheritance errmsg: "child table is missing column ..."

[Vik Fearing]

On 01/07/2017 11:05 PM, Tom Lane wrote:
> Vik Fearing  writes:
>> On 01/07/2017 08:15 PM, Tom Lane wrote:
>>> No, and TBH I would vote strongly against including that much detail in
>>> this error message anyway.  That info could be indefinitely long, and it's
>>> not especially relevant to the stated error condition --- for example, the
>>> presence of a default is *not* relevant to whether the column matches the
>>> parent.  I'm okay with shoehorning column type into this message, but not
>>> much more than that.
> 
>> I agree.
> 
>> Perhaps the ERROR message should remain as is, and a DETAIL or HINT line
>> could be emitted with the entire column definition (or close to it)?
> 
> AFAICS, what Ryan is after would be better served by a separate tool that
> would produce a "diff" of the two tables' schemas.  Even if we were
> willing to overload this error message with everything we know about the
> parent column, do you really want to fix discrepancies one column at a
> time?  And what about properties that can't be uniquely associated with a
> single column, such as constraints covering multiple columns?
> 
> Also, I have a feeling that a suitable tool is already out there.

Well personally, if I were trying to attach one table to another and it
didn't work, I'd use good ol' \d.  Maybe that's just me.
-- 
Vik Fearing  +33 6 46 75 15 36
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support


-- 
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] Adding type info etc for inheritance errmsg: "child table is missing column ..."

[Tom Lane]

Vik Fearing  writes:
> On 01/07/2017 08:15 PM, Tom Lane wrote:
>> No, and TBH I would vote strongly against including that much detail in
>> this error message anyway.  That info could be indefinitely long, and it's
>> not especially relevant to the stated error condition --- for example, the
>> presence of a default is *not* relevant to whether the column matches the
>> parent.  I'm okay with shoehorning column type into this message, but not
>> much more than that.

> I agree.

> Perhaps the ERROR message should remain as is, and a DETAIL or HINT line
> could be emitted with the entire column definition (or close to it)?

AFAICS, what Ryan is after would be better served by a separate tool that
would produce a "diff" of the two tables' schemas.  Even if we were
willing to overload this error message with everything we know about the
parent column, do you really want to fix discrepancies one column at a
time?  And what about properties that can't be uniquely associated with a
single column, such as constraints covering multiple columns?

Also, I have a feeling that a suitable tool is already out there.  A
moment's digging in the list archives found this thread with links to
several candidates:

https://www.postgresql.org/message-id/flat/561D27E7.5010906%40trustport.com

and I'm pretty sure there have been other such discussions.

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] Adding type info etc for inheritance errmsg: "child table is missing column ..."

[Vik Fearing]

On 01/07/2017 08:15 PM, Tom Lane wrote:
> Ryan Murphy  writes:
>> I was hoping for
>> user=# alter table temp inherit entity;
>> ERROR:  child table is missing column "id" uuid default uuid_generate_v1mc()
>> Is there an easy way to get the string that includes all those additional
>> constraints/defaults etc?
> 
> No, and TBH I would vote strongly against including that much detail in
> this error message anyway.  That info could be indefinitely long, and it's
> not especially relevant to the stated error condition --- for example, the
> presence of a default is *not* relevant to whether the column matches the
> parent.  I'm okay with shoehorning column type into this message, but not
> much more than that.

I agree.

Perhaps the ERROR message should remain as is, and a DETAIL or HINT line
could be emitted with the entire column definition (or close to it)?
-- 
Vik Fearing  +33 6 46 75 15 36
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support


-- 
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] Adding type info etc for inheritance errmsg: "child table is missing column ..."

[Ryan Murphy]

> No, and TBH I would vote strongly against including that much detail in
> this error message anyway.  That info could be indefinitely long, and it's
> not especially relevant to the stated error condition --- for example, the
> presence of a default is *not* relevant to whether the column matches the
> parent.  I'm okay with shoehorning column type into this message, but not
> much more than that.
>
> regards, tom lane
>

Ok, that makes sense.  How about things like NOT NULL? you get an error if
your column doesn't have that.

Re: [HACKERS] Adding type info etc for inheritance errmsg: "child table is missing column ..."

[Tom Lane]

Ryan Murphy  writes:
> So I tried using format_type_with_typemod() thinking that the "typemod
> info" meant things like NOT NULL, DEFAULT etc.

No, it means atttypmod, which stores info like the max length for
varchar(n).

> when I was hoping for
> user=# alter table temp inherit entity;
> ERROR:  child table is missing column "id" uuid default uuid_generate_v1mc()
> Is there an easy way to get the string that includes all those additional
> constraints/defaults etc?

No, and TBH I would vote strongly against including that much detail in
this error message anyway.  That info could be indefinitely long, and it's
not especially relevant to the stated error condition --- for example, the
presence of a default is *not* relevant to whether the column matches the
parent.  I'm okay with shoehorning column type into this message, but not
much more than that.

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] Adding type info etc for inheritance errmsg: "child table is missing column ..."

[Ryan Murphy]

> Thanks Tom, I'll redo the patch using one of those, and get back to you
> guys.
>
>
So I tried using format_type_with_typemod() thinking that the "typemod
info" meant things like NOT NULL, DEFAULT etc.  It builds and includes the
plain type but not all that stuff.  E.g.

user=# alter table temp inherit entity;
ERROR:  child table is missing column "id" uuid

when I was hoping for

user=# alter table temp inherit entity;
ERROR:  child table is missing column "id" uuid default uuid_generate_v1mc()

Is there an easy way to get the string that includes all those additional
constraints/defaults etc?  I noticed that defaults seem to be stored in the
Form_pg_attrdef struct defined in src/include/catalog/pg_attrdef.h, and
haven't yet seen how constraints like NOT NULL are handled.

Thanks,
Ryan

Re: [HACKERS] Replication/backup defaults

[Peter Eisentraut]

On 1/7/17 6:23 AM, Magnus Hagander wrote:
> In the build farm, I have found 6 critters that do not end up with the
> 100/128MB setting: sidewinder, curculio, coypu, brolga, lorikeet,
> opossum.  I wonder what limitations initdb is bumping against.
> 
> 
> Since you lookeda t the data -- they did not end up with 100, but what's
> the lowest they did end up with? Did they go all the way down to 10? 

They all ended up on 30 or 40.

The only documented way this could happen is if the semaphore
configuration does not allow enough room, but this would have to be a
very particular setting on all these quite different boxes.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


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

Re: [HACKERS] pg_stat_activity.waiting_start

[Joel Jacobson]

On Sat, Jan 7, 2017 at 3:25 AM, Greg Stark  wrote:
> What users need to know is in aggregate how much of the time the
> database is spending working on their queries is going into different
> states.

This is a separate feature idea, but I think it's really valuable as well.

Maybe something similar to pg_stat_user_functions?
But instead grouping by wait_event_type, wait_event, and showing
accumulated count and sum of waiting time since last stat reset, just
like the other pg_stat_* views?

Maybe something like this?

\d pg_stat_waiting
 View "pg_catalog.pg_stat_waiting"
   Column|   Type   | Modifiers
-+--+---
 wait_event_type | name |
 wait_event  | name |
 waiting_counter | bigint   |
 waiting_time| double precision |


-- 
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] Shrink volume of default make output

[Bruce Momjian]

On Mon, Jan  2, 2017 at 03:37:04PM -0600, Jim Nasby wrote:
> The recent thread about compiler warnings got me thinking about how it's
> essentially impossible to notice warnings with default make output. Perhaps
> everyone just uses make -s by default, though that's a bit annoying since
> you get no output unless something does warn (and then you don't know what
> directory it was in).
> 
> Is it worth looking into this? I'm guessing this may be moot with the CMake
> work, but it's not clear when that'll make it in. In the meantime, ISTM
> http://stackoverflow.com/a/218295 should be an easy change to make (though
> perhaps with a variable that gives you the old behavior).

Please src/tools/pgtest for an example of pulling out warning lines and
reporting them at the end of the build.

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

+ As you are, so once was I.  As I am, so you will be. +
+  Ancient Roman grave inscription +


-- 
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] Adding type info etc for inheritance errmsg: "child table is missing column ..."

[Ryan Murphy]

> The approved way to do this is with format_type_be(), or
> format_type_with_typemod() if you want to include typmod info, which
> I think you probably do for the intended use-case.
>
> regards, tom lane
>

Thanks Tom, I'll redo the patch using one of those, and get back to you
guys.

Re: [HACKERS] Adding type info etc for inheritance errmsg: "child table is missing column ..."

[Tom Lane]

Ryan Murphy  writes:
> The attached patch is my initial attempt at adding the type, making the
> error message read e.g.:
> ERROR: child table is missing column "name" text

"... column "name" of type text", perhaps?  Does not read very nicely
as is.

> I'm sure it needs work (in particular I borrowed a lot of the get-type-name
> logic from getTypeOutputInfo() so probably needs a factor),

The approved way to do this is with format_type_be(), or
format_type_with_typemod() if you want to include typmod info, which
I think you probably do for the intended use-case.

regards, tom lane


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

Re: [HACKERS] [WIP]Vertical Clustered Index (columnar store extension)

[Bruce Momjian]

On Fri, Dec 30, 2016 at 02:55:39PM +1100, Haribabu Kommi wrote:
> 
> Hi All,
> 
> Fujitsu was interested in developing a columnar storage extension with minimal
> changes the server backend.
> 
> The columnar store is implemented as an extension using index access methods.
> This can be easily enhanced with pluggable storage methods once they are
> available.

Have you see this post from 2015:


https://www.postgresql.org/message-id/20150831225328.GM2912%40alvherre.pgsql

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

+ As you are, so once was I.  As I am, so you will be. +
+  Ancient Roman grave inscription +


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

[HACKERS] Adding type info etc for inheritance errmsg: "child table is missing column ..."

[Ryan Murphy]

Hey Postgres team!

I've been gleefully using the inheritance feature of Postgres for the last
6 months or so, and it's really great!  I especially like how easily you
can ALTER TABLE foo INHERIT bar, and get helpful error messages about what
columns need to be there before the inherit can take place.

One thing that seemed helpful to me is if it not only told you that you're
missing the attribute, but also told you the type, and perhaps even the
constraints, so you can easily make the column match up with the one you're
inheriting.  Otherwise you may add the wrong type of column and end up with
a "column is the wrong type" error.

The attached patch is my initial attempt at adding the type, making the
error message read e.g.:

ERROR: child table is missing column "name" text

I'm sure it needs work (in particular I borrowed a lot of the get-type-name
logic from getTypeOutputInfo() so probably needs a factor), and I'd
ultimately love for it to list NOT NULL, DEFAULTs and other constraints to
make it easier to prepare a table to inherit from another.

Thoughts / suggestions?  Does this seem useful to you guys?

Best,
Ryan


0001-child-table-is-missing-attribute-show-type.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] Incorrect XLogRegisterBuffer flag for revmapbuf in brin

[Kuntal Ghosh]

Added to next commitfest for tracking. I should've done it earlier.

-- 
Thanks & Regards,
Kuntal Ghosh
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] Teaching query_planner to handle multiple sort orders?

[Andrew Gierth]

> "Tom" == Tom Lane  writes:

 >> Of course there is one good solution, which is to have query_planner
 >> take a set of acceptable output sort orders rather than just a
 >> single one.

 >> How wild an idea is this?

 Tom> It's been on my to-do list for years, see e.g.
 Tom> https://postgr.es/m/5702.1480896...@sss.pgh.pa.us

 Tom> The idea wouldn't have been very helpful before the upper planner
 Tom> got path-ified, but now the only stumbling block is a shortage of
 Tom> round tuits.

I'll take a shot at it, then.

-- 
Andrew (irc:RhodiumToad)


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

Re: [HACKERS] Teaching query_planner to handle multiple sort orders?

[Tom Lane]

Andrew Gierth  writes:
> So in the grouping sets patch post, I said:
> There is one current weakness which I don't see a good solution for:
> the planner code still has to pick a single value for group_pathkeys
> before planning the input path.  This means that we sometimes can't
> choose a minimal set of sorts, because we may have chosen a sort
> order for a grouping set that should have been hashed,

> Of course there is one good solution, which is to have query_planner
> take a set of acceptable output sort orders rather than just a single
> one.

> How wild an idea is this?

It's been on my to-do list for years, see e.g.
https://postgr.es/m/5702.1480896...@sss.pgh.pa.us
The idea wouldn't have been very helpful before the upper planner got
path-ified, but now the only stumbling block is a shortage of round tuits.

> I guess a possible hazard is an increase in
> the number of possible mergejoin paths to consider.

Yeah, you'd have to keep an eye on possible growth in planning time, but
I feel optimistic that it wouldn't be a big problem.  Usually when you
hear me bitching about planning time growth from some random idea, it's
because the extra cycles would be spent all the time but seldom result in
a win.  But with something like this, extra cycles would be spent only
when there was a demonstrable reason for the investigation, i.e., possible
savings at a grouping or windowing step.  And it wouldn't be a wild-goose
chase but a targeted investigation of directly-relevant sort orders.

regards, tom lane


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

Re: [HACKERS] pg_stat_activity.waiting_start

[Bruce Momjian]

On Sat, Jan  7, 2017 at 01:25:08PM +, Greg Stark wrote:
> I would actually argue the reverse of the above proposal would be more
> useful. What we need are counts of how often LWLocks take longer than,
> say, 50ms and for shorter waits we need to know how long. Perhaps not
> precisely for individual waits but in aggregate we need the totals to
> be right so as long as the measurements are accurate that would be
> sufficient. So an accurate but imprecise measurement +/- 10ms with low
> overhead is better than a precise measurement with high overhead.

I agree those values are important, but I don't think people are going
to be able to use pg_stat_activity to get them, so I don't see the point
of trying to supply them there.

See
https://www.postgresql.org/message-id/ca+tgmoav9q5v5zgt3+wp_1tqjt6tgyxrwrdctrrwimc+zy7...@mail.gmail.com
for an excellent example of getting those values via polling.

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

+ As you are, so once was I.  As I am, so you will be. +
+  Ancient Roman grave inscription +


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

[HACKERS] Teaching query_planner to handle multiple sort orders?

[Andrew Gierth]

So in the grouping sets patch post, I said:

 >> There is one current weakness which I don't see a good solution for:
 >> the planner code still has to pick a single value for group_pathkeys
 >> before planning the input path.  This means that we sometimes can't
 >> choose a minimal set of sorts, because we may have chosen a sort
 >> order for a grouping set that should have been hashed,

Of course there is one good solution, which is to have query_planner
take a set of acceptable output sort orders rather than just a single
one.

How wild an idea is this? I guess a possible hazard is an increase in
the number of possible mergejoin paths to consider. What other possible
issues are there?

-- 
Andrew (irc:RhodiumToad)


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

Re: [HACKERS] pg_background contrib module proposal

[amul sul]

On Sat, Jan 7, 2017 at 2:06 PM, Andrew Borodin  wrote:
> Hi!
>
> 2017-01-07 11:44 GMT+05:00 amul sul :
>
>> Changes:
>> 1. pg_background_launch renamed to pg_background_start
>> 2. pg_background_detach renamed to pg_background_close
>> 3. Added new api to display previously launch background worker & its
>> result count waiting to be read.
> Great work!
>
Thanks.
>
>> Note that attached patches need to apply to the top of the Peter
>> Eisentraut's v2 patch[1].
> I've looked a bit into that patch. I thought you would switch
> MemoryContext before calling BackgroundSessionStart() from
> pg_background? Have I got something wrong?
>
Hmm, understood your point, let pg_background extension do the
required context setup, attached version patch does that change.
Thanks.

Regards,
Amul


0001-Changes-to-Peter-Eisentraut-s-bgsession-v2-patch.patch
Description: Binary data


0002-pg_background-as-client-of-BackgroundSession-v2.patch
Description: Binary data

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

Re: [HACKERS] pg_stat_activity.waiting_start

[Greg Stark]

On 6 January 2017 at 02:59, Bruce Momjian  wrote:
>
> Agreed.  No need in adding overhead for short-lived locks because the
> milli-second values are going to be meaningless to users. I would be
> happy if we could find some weasel value for non-heavyweight locks.

For what it's worth I don't think this is true. It may be true that we
don't need precise measurements of short-lived locks but we do need
accurate measurements even if they're in the expected range.

What users need to know is in aggregate how much of the time the
database is spending working on their queries is going into different
states. Even if no LWLock is held for more than a few milliseconds at
a time if it turns out that 80% of the time is being spend in waiting
on LWLocks then there's no point in worrying about cpu speed, for
example. And knowing *which* LWLock is taking up an aggregate of 80%
of time would point at either configuration changes or code changes to
reduce that contention.

I would actually argue the reverse of the above proposal would be more
useful. What we need are counts of how often LWLocks take longer than,
say, 50ms and for shorter waits we need to know how long. Perhaps not
precisely for individual waits but in aggregate we need the totals to
be right so as long as the measurements are accurate that would be
sufficient. So an accurate but imprecise measurement +/- 10ms with low
overhead is better than a precise measurement with high overhead.

-- 
greg


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

Re: [HACKERS] Make pg_basebackup -x stream the default

[Michael Paquier]

On Sat, Jan 7, 2017 at 12:04 AM, Magnus Hagander  wrote:
> On Wed, Jan 4, 2017 at 10:43 AM, Magnus Hagander 
> wrote:
>> Meh, just as I was going to respond "committed" I noticed this second
>> round of review comments. Apologies, pushed without that.
>>
>> I agree on the change with includewal/streamwal. That was already the case
>> in the existing code of course, but that doesn't mean it couldn't be made
>> better. I'll take a look at doing that as a separate patch.
>>
>
> Here's a patch that does this. Does this match what you were thinking?

Yes, that's it. I have looked at the patch in details and that looks
correct to me.
-- 
Michael


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

Re: [HACKERS] merging some features from plpgsql2 project

[Pavel Stehule]

>
>
> * EXECUTE and FOUND - this is incompatible change, extra check can be used
> (test on unset variable). I see solution in leaving FOUND variable and
> introduction of some new without this issue - ROW_COUNT maybe (this is
> another possible incompatible change, but with higher benefit - maybe we
> can introduce some aliasing, PRAGMA clause, default PRAGMAs, ..).
>

I checked current implementation of FOUND variable. If we introduce new
auto variable ROW_COUNT - exactly like FOUND, then it doesn't introduce any
compatibility break.

ROW_COUNT .. shortcut for GET DIAGNOSTICS row_count = ROW_COUNT.

Comments, notes?

Regards

Pavel

Re: [HACKERS] Replication/backup defaults

[Magnus Hagander]

On Sat, Jan 7, 2017 at 1:27 AM, Peter Eisentraut <
peter.eisentr...@2ndquadrant.com> wrote:

> On 1/5/17 12:01 PM, Andres Freund wrote:
> > On 2017-01-05 08:38:32 -0500, Peter Eisentraut wrote:
> >> I also suggest making the defaults for both 20 instead of 10.  That
> >> leaves enough room that almost nobody ever has to change them, whereas
> >> 10 can be a bit tight for some not-outrageous installations (8 standbys
> >> plus backup?).
> >
> > I'm afraid we need to add initdb integration / testing for those. I mean
> > we have initdb test down to 10 connections to deal with limited
> > resources...
>
> Those initdb defaults were last touched in 2005, before the use of
> System V shared memory was reduced to a minimum.  It might be worth
> revisiting that.  The only way to end up with a low number of connection
> slots would seem to be a very low semaphore configuration.
>
> In the build farm, I have found 6 critters that do not end up with the
> 100/128MB setting: sidewinder, curculio, coypu, brolga, lorikeet,
> opossum.  I wonder what limitations initdb is bumping against.
>
>
Since you lookeda t the data -- they did not end up with 100, but what's
the lowest they did end up with? Did they go all the way down to 10?


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

Re: [HACKERS] Support for pg_receivexlog --format=plain|tar

[Magnus Hagander]

On Sat, Jan 7, 2017 at 12:31 AM, Michael Paquier 
wrote:

> On Fri, Jan 6, 2017 at 11:07 PM, Magnus Hagander 
> wrote:
> > A few further notes:
>
> Thanks for the review.
>
> > You are using the filemode to gzopen and the mode_compression variable to
> > set the compression level. The pre-existing code in pg_basebackup uses
> > gzsetparams(). Is there a particular reason you didn't do it the same
> way?
> >
> > Small comment:
> > -   if (pad_to_size)
> > +   if (pad_to_size && dir_data->compression == 0)
> > {
> > /* Always pre-pad on regular files */
> >
> >
> > That "always" is not true anymore. Commit-time cleanup can be done of
> that.
> >
> > The rest of this looks good to me, but please comment on the gzopen part
> > before we proceed to commit :)
>
> Yes using gzsetparams() looks cleaner. I actually thought about using
> the same logic as pg_dump. Attached is an updated patch.
>
> There is something I forgot. With this patch,
> FindStreamingStart()@pg_receivexlog.c is actually broken. In short it
> forgets to consider files that have been compressed at the last run of
> pg_receivexlog and will try to stream changes from the beginning. I
> can see that gzip -l provides this information... But I have yet to
> find something in zlib that allows a cheap lookup as startup of
> streaming should be fast. Looking at how gzip -l does it may be faster
> than looking at the docs.
>

Do we really care though? As in, does statup of streaming have to be *that*
fast? Even gunziping 16Mb (worst case) doesn't exactly take a long time. If
your pg_receivexlog is restarting so often that it becomes a problem, I
think you already have another and much bigger problem on your hands.

I found another problem with it -- it is completely broken in sync mode.
You need to either forbid sync mode together with compression, or teach
dir_sync() about it. The later would sound better, but I wonder how much
that's going to kill compression due to the small blocks? Is it a
reasonable use-case?


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

Re: [HACKERS] pgbench - allow to store select results into variables

[Fabien COELHO]

Hello Tom,


Please look at changing \into to be a SQL-command-ending backslash
command as we previously discussed.


Done.

There are two variants: \gset & \gcset for compound (end SQL query, set 
variables, but do not end command, so that several settings are allowed in 
a compound command, a key feature for performance testing).


Personnally, I find the end-of-query semicolon-replacing syntax ugly. 
However I'm more interested in feature than in elegance on this one, so 
I'll put up with it.



I think you will find that the implementation is a great deal simpler 
that way and doesn't require weird hackery on the shared lexer.


I have removed the "hackery", only counting embedded semicolons remains to 
keep track of compound queries.


Note that the (somehow buggy and indeed not too clean) hackery was not 
related to the into syntax, but to detecting empty queries which are 
silently skipped by the server.



If you won't do that, [...]


I think that I have done what you required.

I have documented the fact that now the feature does not work if compound 
commands contain empty queries, which is a very minor drawback for a 
pgbench script anyway.


Attached are the patch, a test script for the feature, and various test 
scripts to trigger error cases.


--
Fabien.diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index 3fb29f8..9a0fb12 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -815,6 +815,51 @@ pgbench  options  dbname
   
 
   
+   
+
+ \gcset [prefix] or
+ \gset [prefix]
+
+
+
+ 
+  These commands may be used to end SQL queries, replacing a semicolon.
+  \gcset replaces an embedded semicolon (\;) within
+  a compound SQL command, and \gset replaces a final
+  (;) semicolon which ends the SQL command. 
+ 
+
+ 
+  When these commands are used, the preceding SQL query is expected to
+  return one row, the columns of which are stored into variables named after
+  column names, and prefixed with prefix if provided.
+ 
+
+ 
+  The following example puts the final account balance from the first query
+  into variable abalance, and fills variables
+  one, two and p_three with
+  integers from a compound query.
+
+UPDATE pgbench_accounts
+  SET abalance = abalance + :delta
+  WHERE aid = :aid
+  RETURNING abalance \gset
+-- compound of two queries
+SELECT 1 AS one, 2 AS two \gcset
+SELECT 3 AS three \gset p_
+
+ 
+
+ 
+  
+\gcset and \gset commands do not work when
+empty SQL queries appear within a compound SQL command.
+  
+ 
+
+   
+

 
  \set varname expression
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index f6cb5d4..d57eb31 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -375,11 +375,14 @@ static const char *QUERYMODE[] = {"simple", "extended", "prepared"};
 
 typedef struct
 {
-	char	   *line;			/* text of command line */
+	char	   *line;			/* first line for short display */
+	char	   *lines;			/* full multi-line text of command */
 	int			command_num;	/* unique index of this Command struct */
 	int			type;			/* command type (SQL_COMMAND or META_COMMAND) */
 	int			argc;			/* number of command words */
 	char	   *argv[MAX_ARGS]; /* command word list */
+	int			compound;   /* last compound command (number of \;) */
+	char	  **gset;   /* per-compound command prefix */
 	PgBenchExpr *expr;			/* parsed expression, if needed */
 	SimpleStats stats;			/* time spent in this command */
 } Command;
@@ -1225,6 +1228,105 @@ getQueryParams(CState *st, const Command *command, const char **params)
 		params[i] = getVariable(st, command->argv[i + 1]);
 }
 
+/* read all responses from backend */
+static bool
+read_response(CState *st, char **gset)
+{
+	PGresult   *res;
+	int			compound = -1;
+
+	while ((res = PQgetResult(st->con)) != NULL)
+	{
+		compound += 1;
+
+		switch (PQresultStatus(res))
+		{
+		case PGRES_COMMAND_OK: /* non-SELECT commands */
+		case PGRES_EMPTY_QUERY: /* may be used for testing no-op overhead */
+			if (gset[compound] != NULL)
+			{
+fprintf(stderr,
+		"client %d file %d command %d compound %d: "
+		"\\gset expects a row\n",
+		st->id, st->use_file, st->command, compound);
+st->ecnt++;
+return false;
+			}
+			break; /* OK */
+
+		case PGRES_TUPLES_OK:
+			if (gset[compound] != NULL)
+			{
+/* store result into variables */
+int ntuples = PQntuples(res),
+	nfields = PQnfields(res),
+	f;
+
+if (ntuples != 1)
+{
+	fprintf(stderr,
+			"client %d file %d command %d compound %d: "
+			"expecting one row, got %d\n",
+			st->id, st->use_file, st->command, compound, ntuples);
+	st->ecnt++;
+	PQclear(res);
+	discard_response(st);
+	return false;
+}
+
+for (f = 0; f < nfields ; f++)
+{
+	char *varname = PQfname(res, f);
+	if (*gset[compound] 

Re: [HACKERS] pg_background contrib module proposal

[Andrew Borodin]

Hi!

2017-01-07 11:44 GMT+05:00 amul sul :

> Changes:
> 1. pg_background_launch renamed to pg_background_start
> 2. pg_background_detach renamed to pg_background_close
> 3. Added new api to display previously launch background worker & its
> result count waiting to be read.
Great work!


> Note that attached patches need to apply to the top of the Peter
> Eisentraut's v2 patch[1].
I've looked a bit into that patch. I thought you would switch
MemoryContext before calling BackgroundSessionStart() from
pg_background? Have I got something wrong?

Best regards, Andrey Borodin.


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

Re: [HACKERS] autonomous transactions

[Andrey Borodin]

The following review has been posted through the commitfest application:
make installcheck-world:  tested, passed
Implements feature:   tested, passed
Spec compliant:   tested, passed
Documentation:tested, passed

Here’s review of Background Sessions v2 patch.
===Purpose===
Patch provides API for controlling other backend. Also, patch contains Python 
API support for calling C API.

===Overall status===
Patch applies to current HEAD clearly.
Contains new test and passes existing tests.
Contains sufficient documentation.
Contains 2 TODO items. Not sure it's OK to it leave so.
Another patch from this commitfest (pg_background) is based on this patch.

===Suggestions===
I haven’t found a way to safely acquire status of session (without possibility 
of ereport(ERROR)).
I do not see how to pass massive data, except by one long char* SQL. All the 
values have to be formatted as text.
BackgroundSessionStart() result do not contain PID. This functionality is 
expected by pg_background (though, can be added separately by pg_background). I 
suppose, this is done to prevent API users from accessing internals of 
BackgroundSession structure. But some information have to be public, anyway.
bgsession.c code contains very little comments.
I do not think that switch inside a switch (see bgsession_worker_main()) is 
easy to follow.

===Conclusion===
There’s always something to improve, but I think that this patch is ready for 
committer.

PS. I’ve read the db_link patches, but this review does not apply to them. I 
suppose db_link refactoring would be useful and functionality is added, so I 
think these patches deserve separate commitfest entry.

Best regards, Andrey Borodin.

The new status of this patch is: Ready for Committer

-- 
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] merging some features from plpgsql2 project

[Pavel Stehule]

2017-01-03 17:57 GMT+01:00 Jim Nasby :

> On 1/2/17 1:51 PM, Pavel Stehule wrote:
>
>> 1) Neither is enabled by default, so 90% of users have no idea they
>> exist. Obviously that's an easy enough fix, but...
>>
>> We can strongly talk about it - there can be a chapter in plpgsql doc.
>> Now, the patterns and antipatterns are not officially documented.
>>
>
> Or just fix the issue, provide the backwards compatability GUCs and move
> on.
>
> 2) There's no way to incrementally change those values for a single
>> function. If you've set extra_errors = 'all' globally, a single
>> function can't say "turn off the too many rows setting for this
>> function".
>>
>>
>> We can enhance the GUC syntax like "all -too_many_rows,-xxx"
>>
>
> Why create all that framework when we could just have multiple
> plpgsql.blah GUCs? plpgsql.multirow_assign_level=FATAL solves that
> problem. We just need a plpgsql GUC for each backwards compatibility break.
>
> BTW, while I can see value in being able to change these settings
>> for an entire function, I think the recommended use should be to
>> only change them for a specific statement.
>>
>>
>> What you can do in plain assign statement
>>
>> target := expression ?
>>
>
> The point I was trying to make there is if you do have some cases where
> you need to silently ignore extra rows (for example) it's probably only one
> statement and not an entire function. That said, if we just make these
> options GUCs then you can just do SET and RESET.
>
> My border is any compatibility break - and I would not to across it.
>> First issue is probably harder
>>
>
> If we never broke compatibility we'd still be allowing SELECT without
> FROM, NULL = NULL being TRUE, and a whole bunch of other problems. We'd
> also be stuck on protocol v1 (and of course not talking about what we want
> in v4).
>
> We've successfully made incompatible changes that were *far worse* than
> this (ie: renaming pg_stat_activity.procpid). Obviously we shouldn't be
> breaking things willy-nilly, but these are long-standing warts (dare I say
> BUGS?) that should be fixed. They're ugly enough that someone took the time
> to break plpgsql out of the core code and fork it.


The discussion about changing behave of current features has not a
solution. I don't believe so there is possible to find a win/win solution -
it is not possible. I respect the opinion all people here, but somebody
more afraid of language fragmentation, somebody else more from some
possible production issues. All arguments are valid, all arguments has same
value, all arguments are sent by people with lot of experience (but with
different experience - anybody works with different domains, uses different
patters, hits different kind of errors).

This discussion was +/- about behave of INTO clause - and if STRICT clause
should be default and if STRICT clause should be more strict.

if we introduce new pattern (new syntax), that can be strict enough then we
can go forward. New syntax will not have a impact on current customers code
base. If somebody prefer very strict behave, then he can use new syntax
quickly. Issues in old code can be detected by other tools - plpgsql_check,
and extra_warnings, extra_errors.

Current state
==

INTO
---
* is not strict in any directions - columns or rows
* not equal target and source column number cannot be detected in plpgsql
(plpgsql_check does it)
* missing rows - uses FOUND (PL/SQL), too much rows - uses GET DIAGNOSTICS
x = ROW_COUNT (ANSI/SQL pattern)
* doesn't need outer subtransaction (handled exception) for handling
specific situations

select a into t from test where a = i;
get diagnostics r = row_count;
if r > 1 then raise exception 'too much rows'; end if;

INTO STRICT
--
* is strict in rows - require exactly one row result
* not equal target and source column number cannot be detected in plpgsql
(plpgsql_check does it)
* raises a exceptions no_data_found, too_much_rows
* require outer subtransaction (handled exception) for handling
no_data_found (10% slowdown in worst case)

begin
  select a into t from test where a = i;
exception
  when no_data_found then
t = null;
end;

There is safe pattern (workaround) for missing columns in source - using
RECORD type. Access to missing field raises runtime error. Another solution
- plpgsql_check.

subselect assignment
-
* is column strict - doesn't allow more columns now
* don't allow too_much_rows
* the FOUND is always true - has not change to check it
* although it is ANSI/SQL pattern it is undocumented in PostgreSQL (only
INTO clause is documented)

   x := (select a from test where a = i)

officially unsupported implementation side effect assignment FROM
--
* is undocumented
* allows only one column
* behave is similar like