Re: [HACKERS] trailing comment ghost-timing
On Tue, Dec 24, 2013 at 5:53 AM, Martijn van Oosterhout klep...@svana.org wrote: On Tue, Dec 24, 2013 at 03:40:58AM +0100, Andreas Karlsson wrote: On 12/24/2013 03:17 AM, David Johnston wrote: It is not sent to the server as a trailing comment. The following file is sent to the server like this. File: /**/; /**/ Commands: PQexec(..., /**/;); PQexec(..., /**/); If this has to be fixed it should be in the client. I think people would complain if we broke the API by starting to throw an exception on PQexec with a string containing no actual query. (Untested). Isn't this just a case of psql not printing out a timing if the server responds with PGRES_EMPTY_QUERY? Works... look to the attached patch! Regards, -- Fabrízio de Royes Mello Consultoria/Coaching PostgreSQL Timbira: http://www.timbira.com.br Blog sobre TI: http://fabriziomello.blogspot.com Perfil Linkedin: http://br.linkedin.com/in/fabriziomello Twitter: http://twitter.com/fabriziomello diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c index bbdafab..7320f31 100644 --- a/src/bin/psql/common.c +++ b/src/bin/psql/common.c @@ -1023,12 +1023,12 @@ SendQuery(const char *query) } } - PQclear(results); - /* Possible microtiming output */ - if (pset.timing) + if (pset.timing PQresultStatus(results) != PGRES_EMPTY_QUERY) printf(_(Time: %.3f ms\n), elapsed_msec); + PQclear(results); + /* check for events that may occur during query execution */ if (pset.encoding != PQclientEncoding(pset.db) -- 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] varattno remapping
On 12/24/2013 03:21 PM, Abbas Butt wrote:
Could you please explain a little bit more how would you solve the posed
problem using map_variable_attnos?
I was recently working on a similar problem and used the following algo
to solve it.
I had to find to which column of the base table does a column in
the select statement of the view query belong.
To relate a target list entry in the select query of the view to
an actual column in base table.
Sounds similar. My problem is simplified by the constraint that the view
must be a simple view (only one base relation) and must only contain
simple column-references to single the base relation. No expressions,
no joins. (These are the rules for simply updatable views).
I'm new to the planner and rewriter, so don't take what I do as any kind
of example beyond this seems to work.
I generate a varattno mapping as follows:
/*
* Scan the passed view target list, whose members must consist solely
* of Var nodes with a varno equal to the passed targetvarno.
*
* A mapping is built from the resno (i.e. tlist index) of the view
* tlist to the corresponding attribute number of the base relation
* the varattno points to.
*
* Must not be called with a targetlist containing non-Var entries.
*/
static void
gen_view_base_attr_map(List *viewtlist, AttrNumber * attnomap, int
targetvarno)
{
ListCell*lc;
TargetEntry *te;
Var *tev;
int l_viewtlist = list_length(viewtlist);
foreach(lc, viewtlist)
{
te = (TargetEntry*) lfirst(lc);
/* Could relax this in future and map only the var entries,
* ignoring everything else, but currently pointless since we
* are only interested in simple views. */
Assert(IsA(te-expr, Var));
tev = (Var*) te-expr;
Assert(tev-varno == targetvarno);
Assert(te-resno - 1 l_viewtlist);
attnomap[te-resno - 1] = tev-varattno;
}
}
producing a forward mapping of view attno to base relation attno.
I then apply the varattno remapping, in this case to the returning list
of a DML query acting on a view, with something like:
varattno_map = palloc( list_length(viewquery-targetList) *
sizeof(AttrNumber) );
gen_view_base_attr_map(viewquery-targetList,
varattno_map, rtr-rtindex);
parsetree-returningList = map_variable_attnos(
(Node*) parsetree-returningList,
old_result_rt_index, 0,
varattno_map, list_length(viewquery-targetList),
found_whole_row_var
);
if (found_whole_row_var)
{
/* TODO: Extend map_variable_attnos API to
pass a mutator to handle whole-row vars. */
elog(ERROR, RETURNING list contains a whole-row variable,
which is not currently supported for updatable
views);
}
ChangeVarNodes((Node*) parsetree-returningList,
old_result_rt_index,
new_result_rt_index,
0);
I'd prefer to be doing the map_variable_attnos and ChangeVarNodes work
in a single pass, but it looks like a clumsy and verbose process to
write a new walker. So I'm going to leave it as an opportunity for
future optimisation for now ;-)
(As it happens that found_whole_row_var is a real pain; I'm going to
have to deal with a TODO item in map_variable_attnos to provide a
callback that replaces a whole-row Var with an expansion of it into a
row-expression).
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WITHIN GROUP patch
Sent from my iPad On 24-Dec-2013, at 2:50, Tom Lane t...@sss.pgh.pa.us wrote: Atri Sharma atri.j...@gmail.com writes: Please find attached the latest patch for WITHIN GROUP. This patch is after fixing the merge conflicts. I've committed this after significant editorialization --- most notably, I pushed control of the sort step into the aggregate support functions. I didn't like the way nodeAgg.c had been hacked up to do it the other way. There's a couple hundred lines of code handling that in orderedsetaggs.c, which is more or less comparable to the amount of code that didn't get added to nodeAgg.c, so I think the argument that the original approach avoided code bloat is bogus. The main reason I pushed all the new aggregates into a single file (orderedsetaggs.c) was so they could share a private typedef for the transition state value. It's possible that we should expose that struct so that third-party aggregate functions could leverage the existing transition-function infrastructure instead of having to copy-and-paste it. I wasn't sure where to put it though --- maybe a new include file would be needed. Anyway I left the point for future discussion. regards, tom lane Thank you! -- 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] INSERT...ON DUPLICATE KEY LOCK FOR UPDATE
On 2013-12-23 14:59:31 -0800, Peter Geoghegan wrote: On Mon, Dec 23, 2013 at 7:49 AM, Robert Haas robertmh...@gmail.com wrote: I don't think this is a project to rush through. We've lived without MERGE/UPSERT for several years now, and we can live without it for another release cycle while we try to reach agreement on the way forward. Agreed, but I really think it's one of the biggest weaknesses of postgres at this point. I can tell that you're convinced you know the right way forward here, and you may be right, but I don't think you've convinced everyone else - maybe not even anyone else. That may be. Attention from reviewers has been in relatively short supply. Not that that isn't always true. I don't really see the lack of review as being crucial at this point. At least I have quite some doubts about the approach you've chosen and I have voiced it - so have others. Whether yours is workable seems to hinge entirely on whether you can build a scalable, maintainable value-locking scheme. Besides some thoughts about using slru.c for it I haven't seen much about the design of that part - might just have missed it though. Personally I can't ad-lib a design for it, but I haven't though about it too much. I don't think there's too much reviewers can do before you've provided a POC implementation of real value locking. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] varattno remapping
On 12/24/2013 03:21 PM, Abbas Butt wrote:
Could you please explain a little bit more how would you solve the posed
problem using map_variable_attnos?
It actually turns out to be even simpler, and easy to do in one pass,
when using ReplaceVarsFromTargetList .
You just generate a temporary new list of TargetEntry, with resnos
matching the attribute numbers of the view. Each contained Var points to
the remapped varno and varattno.
Then you let ReplaceVarsFromTargetList substitute Vars recursively
through the expression tree with ones in the replacement tlist you supply.
The more I've thought about it, the shorter this code has got. Currently:
/*
* Scan the passed view target list, whose members must consist solely
* of Var nodes with a varno equal to the passed targetvarno, and
* produce a targetlist of Var nodes with the corresponding varno and
* varattno of the base relation 'targetvarno'.
*
* This tlist is used when remapping Vars that point to a view so they
* point to the base relation of the view instead. It is entirely
* newly allocated. The result tlist is not in resno order.
*
* Must not be called with a targetlist containing non-Var entries.
*/
static List *
gen_view_base_attr_map(List *viewtlist, int targetvarno, int newvarno)
{
ListCell*lc;
TargetEntry *te, *newte;
Var *tev, *newvar;
int l_viewtlist = list_length(viewtlist);
List*newtlist = NIL;
foreach(lc, viewtlist)
{
te = (TargetEntry*) lfirst(lc);
/* Could relax this in future and map only the var entries,
* ignoring everything else, but currently pointless since we
* are only interested in simple views. */
Assert(IsA(te-expr, Var));
tev = (Var*) te-expr;
Assert(tev-varno == targetvarno);
Assert(te-resno - 1 l_viewtlist);
/* Construct the new Var with the remapped attno and varno */
newvar = (Var*) palloc(sizeof(Var));
*newvar = *tev;
newvar-varno = newvarno;
newvar-varattno = tev-varattno;
/* and wrap it in a new tle to cons to the list */
newte = flatCopyTargetEntry(te);
newte-expr = (Expr*) newvar;
newtlist = lcons(newte, newtlist);
}
return newtlist;
}
and invocation:
/*
* We need to adjust any RETURNING clause entries to point to the new
* target RTE instead of the old one so that we see the effects of
* BEFORE triggers. Varattnos must be remapped so that the new Var
* points to the correct col of the base rel, since the view will
* usually have a different set of columns / column order.
*
* As part of this pass any whole-row references to the view are
* expanded into ROW(...) expressions to ensure we don't expose
* columns that are not visible through the view, and to make sure
* the client gets the result type it expected.
*/
List * remap_tlist = gen_view_base_attr_map(
viewquery-targetList, rtr-rtindex, new_result_rt_index);
parsetree-returningList = ReplaceVarsFromTargetList(
(Node*) parsetree-returningList,
old_result_rt_index, 0 /*sublevels_up*/,
view_rte,
remap_tlist,
REPLACEVARS_REPORT_ERROR, 0 /*nomatch_varno, unused */,
NULL /* outer_hasSubLinks, unused */
);
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Assertion failure in base backup code path
On 2013-12-23 18:28:51 +0100, Magnus Hagander wrote: On Dec 19, 2013 12:06 AM, Andres Freund and...@2ndquadrant.com wrote: Hi Magnus, It looks to me like the path to do_pg_start_backup() outside of a transaction context comes from your initial commit of the base backup facility. The problem is that you're not allowed to do anything leading to a syscache/catcache lookup in those contexts. I think that may have come with the addition of the replication privilege actually but that doesn't change the fact that yes, it appears broken.. There was a if (!superuser()) check there before as well, that has the same dangers. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] [bug fix] ECPG app crashes due to SIGBUS on SPARC Solaris
Hello,
I encountered a bug of ECPG with PG 9.2.4, which probably exists in all
releases. The attached patch is for 9.4. Could you review and backport
this to at least 9.2 and later?
[Problem]
The attached ECPG app crashes and dumps core with SIGBUS on Solaris for
SPARC. I used Solaris 10, and Oracle Studio to compile the app for 64-bit
build. The same app completes successfully on Linux and Windows for
x86/x564.
The steps to reproduce the problem is:
1. ecpg sigbus.pgc
2. cc -xtarget=generic64 -Ipgsql_dir/include
sigbus.c -Lpgsql_dir/lib -lecpg
3. a.out
When execting FETCH statement using an SQL descriptor, the app crashes at
the following line in ECPGdo(), which is in
src/interfaces/ecpg/ecpglib/execute.c:
var-value = *((char **) (var-pointer));
[Cause]
ecpg outputs the following line in the preprocessed source file:
{ ECPGdo(__LINE__, 0, 1, NULL, 0, ECPGst_normal, fetch $0,
ECPGt_char,(cur),(long)4,(long)1,(4)*sizeof(char),
ECPGt_NO_INDICATOR, NULL , 0L, 0L, 0L, ECPGt_EOIT,
ECPGt_descriptor, (desc1), 0L, 0L, 0L,
ECPGt_NO_INDICATOR, NULL , 0L, 0L, 0L, ECPGt_EORT);}
So, the above line is executed in ECPGdo(). On the other hand, desc1 is not
aligned on 8-byte boundary. This unaligned access causes SIGBUS.
[Fix]
Because desc1 is a char array, else block should be executed instead of the
above path.
var-value = var-pointer;
Therefore, make ecpg pass SQL descriptor host variables to ECPGdo() with
non-zero lengths.
Regards
MauMau
ECPG_descriptor_crash.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] Planning time in explain/explain analyze
On 12/23/13, 8:12 PM, Andreas Karlsson wrote: A user asked in -performance[1] if there is a way to measure the planning time. log_planner_stats -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix typo in src/backend/utils/mmgr/README
On 12/24/13, 1:33 AM, Etsuro Fujita wrote: This is a small patch to fix a typo in src/backend/utils/mmgr/README. I don't think that change is correct. -- 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] trailing comment ghost-timing
On 12/24/13, 5:40 AM, Fabrízio de Royes Mello wrote: (Untested). Isn't this just a case of psql not printing out a timing if the server responds with PGRES_EMPTY_QUERY? Works... look to the attached patch! That looks reasonable. -- 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] ISN extension bug? (with patch)
On 12/22/13, 2:36 AM, Fabien COELHO wrote: I'm not sure whether the policy is to update the version number of the extension for such a change. As the library is always isn.so, two versions cannot live in parallel anyway. If it is useful, the second patch attached also upgrade the version number. If you are not changing anything in the SQL, then you don't need to change the version number. -- 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] better atomics - v0.2
On 2013-12-23 15:04:08 -0500, Robert Haas wrote: On Thu, Dec 5, 2013 at 6:39 AM, Andres Freund and...@2ndquadrant.com wrote: On 2013-11-19 10:37:35 -0500, Tom Lane wrote: Or does the warning mean code bloat (lots of useless copies of the inline function)? After thinking on that for a bit, yes that's a possible consequence, but it's quite possible that it happens in cases where we don't get the warning too, so I don't think that argument has too much bearing as I don't recall a complaint about it. But I bet the warning we're getting there is complaining about exactly that thing. It's possible that it means warning: i've optimized away your unused function into nothing but I think it's more likely that it means warning: i just emitted dead code. Indeed, I suspect that this is why that test is written that way in the first place. I don't think that's particularly likely - remember, we're talking about static inline functions here. So the compiler would have to detect that there's an inline function which won't be used and then explicitly emit a function body. That seems like an odd thing to do. So, if there's compilers like that around, imo they have to deal with their own problems. It's not like it will prohibit using postgres. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] trailing comment ghost-timing
Hi, On 2013-12-24 02:05:23 +0100, Erik Rijkers wrote: With \timing on, a trailing comment yields a timing. # test.sql select 1; /* select 2 */ $ psql -f test.sql ?column? -- 1 (1 row) Time: 0.651 ms Time: 0.089 ms I assume it is timing something about that comment (right?). Confusing and annoying, IMHO. Is there any chance such trailing ghost-timings can be removed? Maybe I am thinking to technical here, but why would that be a good idea? After all, the comment will have triggered sending a statement to the server and waiting for the result. The user might want to know about that. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] preserving forensic information when we freeze
On 2013-12-23 14:15:29 -0500, Robert Haas wrote: On Mon, Dec 23, 2013 at 1:57 PM, Andres Freund and...@2ndquadrant.com wrote: Well, all of the fundamental changes (combocids, the initial multixact introduction) have been quite some time ago. I think backward compat has a much higher value these days (I also don't see much point in looking at cmin/cmax for anything but diagnostic purposes). I don't think any of the usecases I've seen would be broken by either fk-locks (multixacts have been there before, doesn't matter much that they now contain updates) nor by forensic freezing. The latter because they really only checked that xmin/xmax were the same, and we hopefully haven't broken that... But part of my point really was the usability, not only the performance. Requiring LATERAL queries to be usable sensibly causes a Meh from my side ;) I simply can't imagine that we're going to want to add a system column every time we change something, or even enough of them to cover all of the things we've already got. We'd need at least infomask, infomask2, and hoff, and maybe some functions for peeking through mxacts to find the updater and locker XIDs and lock modes. With a couple of functions we can do all that and not look back. If system columns don't have an overhead anymore, I fail to see the advantage that functions have over simply accessing parts of the row in the normal way parts of rows are accessed. The only reasoning I can see is lessening the likelihood of conflicts - but that's essentially only solved via namespaced pg_ prefixes for functions as well. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] trailing comment ghost-timing
On Tue, December 24, 2013 15:19, Andres Freund wrote: Hi, On 2013-12-24 02:05:23 +0100, Erik Rijkers wrote: With \timing on, a trailing comment yields a timing. # test.sql select 1; /* select 2 */ $ psql -f test.sql ?column? -- 1 (1 row) Time: 0.651 ms Time: 0.089 ms I assume it is timing something about that comment (right?). Confusing and annoying, IMHO. Is there any chance such trailing ghost-timings can be removed? Maybe I am thinking to technical here, but why would that be a good idea? After all, the comment will have triggered sending a statement to the server and waiting for the result. The user might want to know about that. Technical or non-technical; it's at least pretty inconsistent: - it only times with the last comment. - it only times with the /**/ comment; to time your trailing -- comments you'll have to find another solution :) Obviously it's a minor thing, and I don't care if it does not get removed, but I don't think you can argue that it serves any useful purpose in the current state. Erik Rijkers -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] trailing comment ghost-timing
Andres Freund and...@2ndquadrant.com writes: Maybe I am thinking to technical here, but why would that be a good idea? After all, the comment will have triggered sending a statement to the server and waiting for the result. The user might want to know about that. I agree; if we triggered a server operation, we should report a timing. Pretending we did not isn't a good solution. The real question is whether we shouldn't suppress the whole PQexec. I believe this is very closely related to the question of what we do with a comment preceding the next command. Try this experiment: regression=# set log_statement = 'all'; SET regression=# /* block comment here */ regression-# select 2+2; ?column? -- 4 (1 row) regression=# -- dash comment here regression=# select 3+3; ?column? -- 6 (1 row) and then look in the postmaster log: LOG: statement: /* block comment here */ select 2+2; LOG: statement: select 3+3; This is inconsistent, IMO. I think if we were to fix things so that leading block comments were dropped the same way -- comments are, that would also take care of the behavior complained of in this thread. There's been some previous discussion of this point, I think. A related issue is that psql suppresses -- comments after the start of the SQL statement as well as before it. I think that this is probably not such a good idea anymore; it has the potential at least to screw up psql's reporting of error locations. I think what would likely be the ideal behavior is to drop all leading lines containing only whitespace/comments, but once we identify the first line of the statement, transmit verbatim up till the closing semicolon. Fun corner case: /* block comment here */ SELECT ... If we try to strip this comment we're definitely going to have issues with error cursor positions. So probably the way it will have to work is to make a check at each end-of-line about whether we are outside any block comment and haven't seen any SQL token yet, and flush the query collection buffer if so. 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] varattno remapping
On 24 December 2013 12:12, Craig Ringer cr...@2ndquadrant.com wrote:
On 12/24/2013 03:21 PM, Abbas Butt wrote:
Could you please explain a little bit more how would you solve the posed
problem using map_variable_attnos?
It actually turns out to be even simpler, and easy to do in one pass,
when using ReplaceVarsFromTargetList .
You just generate a temporary new list of TargetEntry, with resnos
matching the attribute numbers of the view. Each contained Var points to
the remapped varno and varattno.
Then you let ReplaceVarsFromTargetList substitute Vars recursively
through the expression tree with ones in the replacement tlist you supply.
The more I've thought about it, the shorter this code has got. Currently:
/*
* Scan the passed view target list, whose members must consist solely
* of Var nodes with a varno equal to the passed targetvarno, and
* produce a targetlist of Var nodes with the corresponding varno and
* varattno of the base relation 'targetvarno'.
*
* This tlist is used when remapping Vars that point to a view so they
* point to the base relation of the view instead. It is entirely
* newly allocated. The result tlist is not in resno order.
*
* Must not be called with a targetlist containing non-Var entries.
*/
static List *
gen_view_base_attr_map(List *viewtlist, int targetvarno, int newvarno)
{
ListCell*lc;
TargetEntry *te, *newte;
Var *tev, *newvar;
int l_viewtlist = list_length(viewtlist);
List*newtlist = NIL;
foreach(lc, viewtlist)
{
te = (TargetEntry*) lfirst(lc);
/* Could relax this in future and map only the var entries,
* ignoring everything else, but currently pointless since we
* are only interested in simple views. */
Assert(IsA(te-expr, Var));
tev = (Var*) te-expr;
Assert(tev-varno == targetvarno);
Assert(te-resno - 1 l_viewtlist);
/* Construct the new Var with the remapped attno and varno */
newvar = (Var*) palloc(sizeof(Var));
*newvar = *tev;
newvar-varno = newvarno;
newvar-varattno = tev-varattno;
/* and wrap it in a new tle to cons to the list */
newte = flatCopyTargetEntry(te);
newte-expr = (Expr*) newvar;
newtlist = lcons(newte, newtlist);
}
return newtlist;
}
I don't think this bit is quite right.
It's not correct to assume that all the view columns are simple
references to columns of the base relation --- auto-updatable views
may now contain a mix of updatable and non-updatable columns, so some
of the view columns may be arbitrary expressions.
There is already code in rewriteTargetView() that does something very
similar (to the whole parsetree, rather than just the returning list)
with 2 function calls:
/*
* Make a copy of the view's targetlist, adjusting its Vars to reference
* the new target RTE, ie make their varnos be new_rt_index instead of
* base_rt_index. There can be no Vars for other rels in the tlist, so
* this is sufficient to pull up the tlist expressions for use in the
* outer query. The tlist will provide the replacement expressions used
* by ReplaceVarsFromTargetList below.
*/
view_targetlist = copyObject(viewquery-targetList);
ChangeVarNodes((Node *) view_targetlist,
base_rt_index,
new_rt_index,
0);
and then later:
/*
* Now update all Vars in the outer query that reference the view to
* reference the appropriate column of the base relation instead.
*/
parsetree = (Query *)
ReplaceVarsFromTargetList((Node *) parsetree,
parsetree-resultRelation,
0,
view_rte,
view_targetlist,
REPLACEVARS_REPORT_ERROR,
0,
parsetree-hasSubLinks);
Regards,
Dean
and invocation:
/*
* We need to adjust any RETURNING clause entries to point to the new
* target RTE instead of the old one so that we see the effects of
* BEFORE triggers. Varattnos must be remapped so that the new Var
* points to the correct col of the base rel, since the view will
* usually have a different set of columns / column order.
*
* As part of this pass any whole-row references to the view are
* expanded into ROW(...) expressions to ensure we don't expose
* columns that are not visible through the view, and to make sure
* the client gets the result type it expected.
*/
List * remap_tlist = gen_view_base_attr_map(
viewquery-targetList, rtr-rtindex, new_result_rt_index);
parsetree-returningList = ReplaceVarsFromTargetList(
(Node*)
Re: [HACKERS] trailing comment ghost-timing
The real question is whether we shouldn't suppress the whole PQexec. I believe this is very closely related to the question of what we do with a comment preceding the next command. Try this experiment: regression=# /* block comment here */ regression-# select 2+2; regression=# -- dash comment here regression=# select 3+3; and then look in the postmaster log: LOG: statement: /* block comment here */ select 2+2; LOG: statement: select 3+3; This is inconsistent, IMO. I think if we were to fix things so that leading block comments were dropped the same way -- comments are, that would also take care of the behavior complained of in this thread. There's been some previous discussion of this point, I think. FWIW, I find dropping comments a rather annoying behaviour. I'd rather include dash comments in the statements sent to the server than start dropping block comments. It's not uncommon to annotate statements with additional information using comments for log analysis. Sure, most of that isn't sent via psql, but it's imo still surpising when testing or re-executing stuff from the log. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [bug fix] pg_ctl stop times out when it should respond quickly
On 12/5/13, 7:07 AM, MauMau wrote: From: Tom Lane t...@sss.pgh.pa.us If you're going to do a postmaster_is_alive check, why bother with repeated get_pgpid()? As I said yesterday, I removed get_pgpid() calls. I'll add this patch to 2014-1 commitfest this weekend if it is not committed until then. This patch breaks the regression tests: xml ... ok test stats... ok == shutting down postmaster == waits a long time pg_ctl: server does not shut down pg_regress: could not stop postmaster: exit code was 256 -- 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] GIN improvements part 1: additional information
On 12/10/13, 2:44 PM, Alexander Korotkov wrote: However, patch didn't apply to head. Corrected version is attached. Update the pgstattuple regression test results. -- 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] ISN extension bug? (with patch)
On 12/22/13, 2:36 AM, Fabien COELHO wrote: I'm not sure whether the policy is to update the version number of the extension for such a change. As the library is always isn.so, two versions cannot live in parallel anyway. If it is useful, the second patch attached also upgrade the version number. If you are not changing anything in the SQL, then you don't need to change the version number. Ok, thanks for the information. I understand that the version number is about the API, not the implementation. If so, there is only the one-liner patch to consider. -- Fabien. -- 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] Problem with displaying wide tables in psql
Please fix this: src/bin/psql/print.c:1269: trailing whitespace. src/bin/psql/print.c:1351: trailing whitespace. src/bin/psql/print.c:1359: trailing whitespace. src/bin/psql/print.c:1364: trailing whitespace. src/bin/psql/print.c:2263: trailing whitespace. -- 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] [COMMITTERS] pgsql: Upgrade to Autoconf 2.69
Alvaro Herrera wrote: Heikki, Andres, Shortly after this patch was committed, buildfarm member locust (running Mac OS X 10.5 apparently) started failing the pg_upgrade check: command: /Users/pgbuildfarm/Documents/workdir/HEAD/pgsql.82393/contrib/pg_upgrade/tmp_check/install//Users/pgbuildfarm/Documents/workdir//HEAD/inst/bin/pg_ctl -w -l pg_upgrade_server.log -D /Users/pgbuildfarm/Documents/workdir/HEAD/pgsql.82393/contrib/pg_upgrade/tmp_check/data -o -p 57632 -b -c synchronous_commit=off -c fsync=off -c full_page_writes=off -c listen_addresses='' -c unix_socket_permissions=0700 -c unix_socket_directories='/Users/pgbuildfarm/Documents/workdir/HEAD/pgsql.82393/contrib/pg_upgrade' start pg_upgrade_server.log 21 waiting for server to startLOG: database system was shut down at 2013-12-19 12:51:16 CET LOG: invalid primary checkpoint record LOG: invalid secondary checkpoint link in control file PANIC: could not locate a valid checkpoint record Any comment on this problem? Somehow ReadRecord is unable to find a checkpoint, yet there's no error message to be seen anywhere, whereas pg_resetxlog does report it: command: /Users/pgbuildfarm/Documents/workdir/HEAD/pgsql.82393/contrib/pg_upgrade/tmp_check/install//Users/pgbuildfarm/Documents/workdir//HEAD/inst/bin/pg_resetxlog -l 00010009 /Users/pgbuildfarm/Documents/workdir/HEAD/pgsql.82393/contrib/pg_upgrade/tmp_check/data pg_upgrade_utility.log 21 pg_resetxlog: could not read from directory pg_xlog: Invalid argument I cannot but think xlogreader is at fault. Regardless of the solution to the Mac OS X problem, ISTM this should be fixed. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [COMMITTERS] pgsql: Upgrade to Autoconf 2.69
Hi, On 2013-12-24 12:58:04 -0300, Alvaro Herrera wrote: Shortly after this patch was committed, buildfarm member locust (running Mac OS X 10.5 apparently) started failing the pg_upgrade check: command: /Users/pgbuildfarm/Documents/workdir/HEAD/pgsql.82393/contrib/pg_upgrade/tmp_check/install//Users/pgbuildfarm/Documents/workdir//HEAD/inst/bin/pg_ctl -w -l pg_upgrade_server.log -D /Users/pgbuildfarm/Documents/workdir/HEAD/pgsql.82393/contrib/pg_upgrade/tmp_check/data -o -p 57632 -b -c synchronous_commit=off -c fsync=off -c full_page_writes=off -c listen_addresses='' -c unix_socket_permissions=0700 -c unix_socket_directories='/Users/pgbuildfarm/Documents/workdir/HEAD/pgsql.82393/contrib/pg_upgrade' start pg_upgrade_server.log 21 waiting for server to startLOG: database system was shut down at 2013-12-19 12:51:16 CET LOG: invalid primary checkpoint record LOG: invalid secondary checkpoint link in control file PANIC: could not locate a valid checkpoint record Any comment on this problem? Somehow ReadRecord is unable to find a checkpoint, yet there's no error message to be seen anywhere, whereas pg_resetxlog does report it: command: /Users/pgbuildfarm/Documents/workdir/HEAD/pgsql.82393/contrib/pg_upgrade/tmp_check/install//Users/pgbuildfarm/Documents/workdir//HEAD/inst/bin/pg_resetxlog -l 00010009 /Users/pgbuildfarm/Documents/workdir/HEAD/pgsql.82393/contrib/pg_upgrade/tmp_check/data pg_upgrade_utility.log 21 pg_resetxlog: could not read from directory pg_xlog: Invalid argument I cannot but think xlogreader is at fault. Regardless of the solution to the Mac OS X problem, ISTM this should be fixed. I didn't look at any code, and I won't today, but it doesn't look surprising - the report when starting the server above is presumable the one in ReadCheckpoint() (or similar) and it probably just reports that ReadRecord() didn't return a record. pg_resetxlog (which doesn't use xlogreader!) reports that it couldn't read from directory pg_xlog, so there's something wonky independently from xlogreader. I'd guess that xlog.c read_page callback errors out without reporting an error. IIRC we're logging some failures as DEBUG there, because they really aren't unexpected, and normally just signal the end of wal. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Problem with displaying wide tables in psql
fixed
2013/12/24 Peter Eisentraut pete...@gmx.net
Please fix this:
src/bin/psql/print.c:1269: trailing whitespace.
src/bin/psql/print.c:1351: trailing whitespace.
src/bin/psql/print.c:1359: trailing whitespace.
src/bin/psql/print.c:1364: trailing whitespace.
src/bin/psql/print.c:2263: trailing whitespace.
--
Best regards,
Sergey Muraviov
From be9f01777599dc5e84c417e5cae56459677a88d4 Mon Sep 17 00:00:00 2001
From: Sergey Muraviov sergey.k.murav...@gmail.com
Date: Wed, 11 Dec 2013 20:17:26 +0400
Subject: [PATCH 1/2] wrapped tables in expanded mode
---
src/bin/psql/print.c | 123 ---
1 file changed, 118 insertions(+), 5 deletions(-)
diff --git a/src/bin/psql/print.c b/src/bin/psql/print.c
index 736225c..4c37f7d 100644
--- a/src/bin/psql/print.c
+++ b/src/bin/psql/print.c
@@ -124,6 +124,7 @@ const printTextFormat pg_utf8format =
/* Local functions */
static int strlen_max_width(unsigned char *str, int *target_width, int encoding);
+static bool IsWrappingNeeded(const printTableContent *cont, bool is_pager);
static void IsPagerNeeded(const printTableContent *cont, const int extra_lines, bool expanded,
FILE **fout, bool *is_pager);
@@ -1234,6 +1235,45 @@ print_aligned_vertical(const printTableContent *cont, FILE *fout)
fprintf(fout, %s\n, cont-title);
}
+ if (IsWrappingNeeded(cont, is_pager))
+ {
+ int output_columns = 0;
+ /*
+ * Choose target output width: \pset columns, or $COLUMNS, or ioctl
+ */
+ if (cont-opt-columns 0)
+ output_columns = cont-opt-columns;
+ else
+ {
+ if (cont-opt-env_columns 0)
+output_columns = cont-opt-env_columns;
+#ifdef TIOCGWINSZ
+ else
+ {
+struct winsize screen_size;
+
+if (ioctl(fileno(stdout), TIOCGWINSZ, screen_size) != -1)
+ output_columns = screen_size.ws_col;
+ }
+#endif
+ }
+
+ output_columns -= hwidth;
+
+ if (opt_border == 0)
+ output_columns -= 1;
+ else
+ {
+ output_columns -= 3; /* -+- */
+
+ if (opt_border 1)
+output_columns -= 4; /* +--+ */
+ }
+
+ if ((output_columns 0) (dwidth output_columns))
+ dwidth = output_columns;
+ }
+
/* print records */
for (i = 0, ptr = cont-cells; *ptr; i++, ptr++)
{
@@ -1294,12 +1334,49 @@ print_aligned_vertical(const printTableContent *cont, FILE *fout)
if (!dcomplete)
{
-if (opt_border 2)
- fprintf(fout, %s\n, dlineptr[line_count].ptr);
+if (dlineptr[line_count].width dwidth)
+{
+ int offset = 0;
+ int chars_to_output = dlineptr[line_count].width;
+ while (chars_to_output 0)
+ {
+ int target_width, bytes_to_output;
+
+ if (offset 0)
+ {
+ if (opt_border == 2)
+fprintf(fout, %s , dformat-leftvrule);
+
+ fprintf(fout, %*s, hwidth, );
+
+ if (opt_border 0)
+fprintf(fout, %s , dformat-midvrule);
+ else
+fputc(' ', fout);
+ }
+
+ target_width = dwidth;
+ bytes_to_output = strlen_max_width(dlineptr[line_count].ptr + offset,
+ target_width, encoding);
+ fputnbytes(fout, (char *)(dlineptr[line_count].ptr + offset), bytes_to_output);
+ chars_to_output -= target_width;
+ offset += bytes_to_output;
+
+ if (opt_border 2)
+ fputc('\n', fout);
+ else
+ fprintf(fout, %*s %s\n, dwidth - target_width, , dformat-rightvrule);
+ }
+}
else
- fprintf(fout, %-s%*s %s\n, dlineptr[line_count].ptr,
- dwidth - dlineptr[line_count].width, ,
- dformat-rightvrule);
+{
+ if (opt_border 2)
+ fprintf(fout, %s\n, dlineptr[line_count].ptr);
+ else
+ fprintf(fout, %-s%*s %s\n, dlineptr[line_count].ptr,
+dwidth - dlineptr[line_count].width, ,
+dformat-rightvrule);
+}
if (!dlineptr[line_count + 1].ptr)
dcomplete = 1;
@@ -2175,6 +2252,42 @@ print_troff_ms_vertical(const printTableContent *cont, FILE *fout)
/* Public functions */
//
+/*
+ * IsWrappingNeeded
+ *
+ * Tests if wrapping is needed
+ */
+static bool
+IsWrappingNeeded(const printTableContent *cont, bool is_pager)
+{
+ const char *pagerprog = 0,
+*less_options = 0;
+
+ if ((cont-opt-format == PRINT_WRAPPED) || (is_pager == false))
+ return true;
+
+ pagerprog = getenv(PAGER);
+ if (!pagerprog)
+ pagerprog = DEFAULT_PAGER;
+
+ if (strcmp(pagerprog, less) != 0)
+ return true;
+
+ less_options = getenv(LESS);
+ if (!less_options)
+ return true;
+/*
+ * Test for -S option
+ * Causes lines longer than the screen width to be chopped rather
+ * than folded. That is, the portion of a long line that does not
+ * fit in the screen width is not shown. The default is to fold
+ * long lines; that is, display the remainder on the next line.
+ */
+ if (strchr(less_options, 'S'))
+ return false;
+ else
+ return true;
+}
/*
* PageOutput
--
1.8.4.2
From 9c4076796386ee3062fcc51272eb3e6e9b66bb1d Mon Sep 17 00:00:00 2001
From: Sergey Muraviov
Re: [HACKERS] trailing comment ghost-timing
Andres Freund and...@2ndquadrant.com writes: This is inconsistent, IMO. I think if we were to fix things so that leading block comments were dropped the same way -- comments are, that would also take care of the behavior complained of in this thread. There's been some previous discussion of this point, I think. FWIW, I find dropping comments a rather annoying behaviour. I'd rather include dash comments in the statements sent to the server than start dropping block comments. What I was proposing was that we do include comments in what we send, as long as those comments are embedded in the statement text, not on lines before it. 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] Logging WAL when updating hintbit
On Tue, Dec 24, 2013 at 1:58 PM, Michael Paquier michael.paqu...@gmail.com wrote: On Sat, Dec 21, 2013 at 4:13 AM, Robert Haas robertmh...@gmail.com wrote: On Fri, Dec 20, 2013 at 9:06 AM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Michael Paquier escribió: On Fri, Dec 20, 2013 at 1:05 PM, Sawada Masahiko sawada.m...@gmail.com wrote: Sorry the patch which I attached has wrong indent on pg_controldata. I have modified it and attached the new version patch. Now that you send this patch, I am just recalling some recent email from Tom arguing about avoiding to mix lower and upper-case characters for a GUC parameter name: http://www.postgresql.org/message-id/30569.1384917...@sss.pgh.pa.us To fullfill this requirement, could you replace walLogHints by wal_log_hints in your patch? Thoughts from others? The issue is with the user-visible variables, not with internal variables implementing them. I think the patch is sane. (Other than the fact that it was posted as a patch-on-patch instead of patch-on-master). But spelling it the same way everywhere really improves greppability. Yep, finding all the code paths with a single keyword is usually a good thing. Attached is a purely-aesthetical patch that updates the internal variable name to wal_log_hints. There are many GUC parameters other than wal_log_hints, that their names are not the same as the names of their variables. We should update also them? Regards, -- Fujii Masao -- 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] ALTER SYSTEM SET command to change postgresql.conf parameters
On Tue, Dec 24, 2013 at 2:57 AM, Amit Kapila amit.kapil...@gmail.com wrote: On Sun, Dec 22, 2013 at 8:32 PM, Amit Kapila amit.kapil...@gmail.com wrote: On Sun, Dec 22, 2013 at 3:01 PM, Fujii Masao masao.fu...@gmail.com wrote: I found the bug of ALTER SYSTEM SET patch. The procedure to reproduce it is here. $ psql =# ALTER SYSTEM SET shared_buffers = '512MB'; ALTER SYSTEM =# \q $ pg_ctl -D data reload server signaled 2013-12-22 18:24:13 JST LOG: received SIGHUP, reloading configuration files 2013-12-22 18:24:13 JST LOG: parameter shared_buffers cannot be changed without restarting the server 2013-12-22 18:24:13 JST LOG: configuration file X?? contains errors; unaffected changes were applied The configuration file name in the last log message is broken. This problem was introduced by the ALTER SYSTEM SET patch. FreeConfigVariables(head); snip else if (apply) ereport(elevel, (errcode(ERRCODE_CONFIG_FILE_ERROR), errmsg(configuration file \%s\ contains errors; unaffected changes were applied, ErrorConfFile))); The cause of the problem is that, in ProcessConfigFile(), the log message including the 'ErrorConfFile' is emitted after 'head' is free'd even though 'ErrorConfFile' points to one of entry of the 'head'. Your analysis is absolutely right. The reason for this issue is that we want to display filename to which erroneous parameter belongs and in this case we are freeing the parameter info before actual error. To fix, we need to save the filename value before freeing parameter list. Please find the attached patch to fix this issue. Note - In my m/c, I could not reproduce the issue. I think this is not surprising because we are not setting freed memory to NULL, so it can display anything (sometimes right value as well) If you find that the provided patch doesn't fix the problem or you don't find it appropriate due to some other reason, let me know the feedback. When I read ProcessConfigFile() more carefully, I found another related problem. The procedure to reproduce the problem is here. $ psql =# ALTER SYSTEM SET shared_buffers = '256MB'; =# \q $ echo shared_buffers = '256MB' $PGDATA/postgresql.conf $ pg_ctl -D $PGDATA reload 2013-12-25 03:05:44 JST LOG: parameter shared_buffers cannot be changed without restarting the server 2013-12-25 03:05:44 JST LOG: parameter shared_buffers cannot be changed without restarting the server 2013-12-25 03:05:44 JST LOG: configuration file /dav/alter-system/data/postgresql.auto.conf contains errors; unaffected changes were applied Both postgresql.conf and postgresql.auto.conf contain the setting of the parameter that cannot be changed without restarting the server. But only postgresql.auto.conf was logged, and which would be confusing, I'm afraid. I think that this problem should be fixed together with the problem that I reported before. Thought? Regards, -- Fujii Masao -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] CREATE TABLESPACE SET
I was recently annoyed that I had to do
CREATE TABLESPACE x LOCATION y;
ALTER TABLESPACE x SET (random_page_cost = z);
The attached patch is a quick n' dirty extension to allow the SET
directly on the CREATE statement.
CREATE TABLESPACE x LOCATION y SET (random_page_cost = z);
If people think this is a good idea, I'll clean it up, add
documentation, and submit it to the next commitfest.
--
Vik
*** a/src/backend/commands/tablespace.c
--- b/src/backend/commands/tablespace.c
***
*** 234,239 CreateTableSpace(CreateTableSpaceStmt *stmt)
--- 234,240
Oid tablespaceoid;
char *location;
Oid ownerId;
+ Datum newOptions;
/* Must be super user */
if (!superuser())
***
*** 317,323 CreateTableSpace(CreateTableSpaceStmt *stmt)
values[Anum_pg_tablespace_spcowner - 1] =
ObjectIdGetDatum(ownerId);
nulls[Anum_pg_tablespace_spcacl - 1] = true;
! nulls[Anum_pg_tablespace_spcoptions - 1] = true;
tuple = heap_form_tuple(rel-rd_att, values, nulls);
--- 318,333
values[Anum_pg_tablespace_spcowner - 1] =
ObjectIdGetDatum(ownerId);
nulls[Anum_pg_tablespace_spcacl - 1] = true;
!
! /* Generate new proposed spcoptions (text array) */
! newOptions = transformRelOptions((Datum) 0,
! stmt-options,
! NULL, NULL, false, false);
! (void) tablespace_reloptions(newOptions, true);
! if (newOptions != (Datum) 0)
! values[Anum_pg_tablespace_spcoptions - 1] = newOptions;
! else
! nulls[Anum_pg_tablespace_spcoptions - 1] = true;
tuple = heap_form_tuple(rel-rd_att, values, nulls);
*** a/src/backend/nodes/copyfuncs.c
--- b/src/backend/nodes/copyfuncs.c
***
*** 3370,3375 _copyCreateTableSpaceStmt(const CreateTableSpaceStmt *from)
--- 3370,3376
COPY_STRING_FIELD(tablespacename);
COPY_STRING_FIELD(owner);
COPY_STRING_FIELD(location);
+ COPY_NODE_FIELD(options);
return newnode;
}
*** a/src/backend/nodes/equalfuncs.c
--- b/src/backend/nodes/equalfuncs.c
***
*** 1610,1615 _equalCreateTableSpaceStmt(const CreateTableSpaceStmt *a, const CreateTableSpace
--- 1610,1616
COMPARE_STRING_FIELD(tablespacename);
COMPARE_STRING_FIELD(owner);
COMPARE_STRING_FIELD(location);
+ COMPARE_NODE_FIELD(options);
return true;
}
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
***
*** 325,331 static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
%type list stmtblock stmtmulti
OptTableElementList TableElementList OptInherit definition
OptTypedTableElementList TypedTableElementList
! reloptions opt_reloptions
OptWith opt_distinct opt_definition func_args func_args_list
func_args_with_defaults func_args_with_defaults_list
aggr_args aggr_args_list
--- 325,331
%type list stmtblock stmtmulti
OptTableElementList TableElementList OptInherit definition
OptTypedTableElementList TypedTableElementList
! reloptions opt_reloptions opt_setreloptions
OptWith opt_distinct opt_definition func_args func_args_list
func_args_with_defaults func_args_with_defaults_list
aggr_args aggr_args_list
***
*** 2275,2280 opt_reloptions: WITH reloptions { $$ = $2; }
--- 2275,2284
| /* EMPTY */ { $$ = NIL; }
;
+ opt_setreloptions: SET reloptions { $$ = $2; }
+ | /* EMPTY */ { $$ = NIL; }
+ ;
+
reloption_list:
reloption_elem { $$ = list_make1($1); }
| reloption_list ',' reloption_elem { $$ = lappend($1, $3); }
***
*** 3588,3599 opt_procedural:
*
*/
! CreateTableSpaceStmt: CREATE TABLESPACE name OptTableSpaceOwner LOCATION Sconst
{
CreateTableSpaceStmt *n = makeNode(CreateTableSpaceStmt);
n-tablespacename = $3;
n-owner = $4;
n-location = $6;
$$ = (Node *) n;
}
;
--- 3592,3604
*
*/
! CreateTableSpaceStmt: CREATE TABLESPACE name OptTableSpaceOwner LOCATION Sconst opt_setreloptions
{
CreateTableSpaceStmt *n = makeNode(CreateTableSpaceStmt);
n-tablespacename = $3;
n-owner = $4;
n-location = $6;
+ n-options = $7;
$$ = (Node *) n;
}
;
*** a/src/include/nodes/parsenodes.h
--- b/src/include/nodes/parsenodes.h
***
*** 1669,1674 typedef struct CreateTableSpaceStmt
--- 1669,1675
char *tablespacename;
char *owner;
char *location;
+ List *options;
} CreateTableSpaceStmt;
typedef struct DropTableSpaceStmt
--
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] sepgsql: label regression test failed
Hello,
It seems to me changes in the base security policy on Fedora affected to
the regression test. Our test cases for sepgsql_setcon() utilizes the MCS
rules, that prevents domain transition from narrow categories to wider ones,
to control the success cases and failure cases.
However, its coverage was changed. It was applied all the domains in the
system, thus unconfined_t domain had been enforced by MCS rules.
But now, it shall be applied only domains with mcs_constrained_type
attribute.
[kaigai@vmlinux tmp]$ diff -up old/policy/mcs new/policy/mcs
:
snip
:
mlsconstrain process { transition dyntransition }
- (( h1 dom h2 ) or ( t1 == mcssetcats ));
+ (( h1 dom h2 ) or ( t1 != mcs_constrained_type ));
Probably, we need to define a domain by ourselves for regression test to ensure
the test stability, not using the system unconfined domain that has different
meaning by release.
I'll make a patch. Please wait for a while.
Thanks for your test reports.
2013/12/18 Sergey Muraviov sergey.k.murav...@gmail.com:
# semodule -l | grep sepgslq
sepgsql-regtest 1.07
Full list of modules is in attachment.
2013/12/18 Kohei KaiGai kai...@kaigai.gr.jp
Could you show me semodule -l on your environment?
I believe security policy has not been changed between F19 and F20...
Thanks,
2013/12/18 Sergey Muraviov sergey.k.murav...@gmail.com:
Hi
I've tried to test postgres 9.3.2 and 9.4devel with selinux on Fedora 20
and
met with a label regression test failure.
PS
I've got some warning during build process.
--
Best regards,
Sergey Muraviov
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
--
KaiGai Kohei kai...@kaigai.gr.jp
--
Best regards,
Sergey Muraviov
--
KaiGai Kohei kai...@kaigai.gr.jp
--
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] [bug fix] ECPG app crashes due to SIGBUS on SPARC Solaris
2013-12-24 13:55 kelteze'ssel, MauMau i'rta:
Hello,
I encountered a bug of ECPG with PG 9.2.4, which probably exists in all
releases. The
attached patch is for 9.4. Could you review and backport this to at least 9.2
and later?
[Problem]
The attached ECPG app
The app wasn't attached, only the patch.
If this is a small test app, it can also be a part of the patch in the form of a
regression test.
crashes and dumps core with SIGBUS on Solaris for SPARC. I used Solaris 10,
and Oracle
Studio to compile the app for 64-bit build. The same app completes
successfully on Linux
and Windows for x86/x564.
The steps to reproduce the problem is:
1. ecpg sigbus.pgc
2. cc -xtarget=generic64 -Ipgsql_dir/include sigbus.c -Lpgsql_dir/lib
-lecpg
3. a.out
When execting FETCH statement using an SQL descriptor, the app crashes at the
following
line in ECPGdo(), which is in src/interfaces/ecpg/ecpglib/execute.c:
var-value = *((char **) (var-pointer));
[Cause]
ecpg outputs the following line in the preprocessed source file:
{ ECPGdo(__LINE__, 0, 1, NULL, 0, ECPGst_normal, fetch $0,
ECPGt_char,(cur),(long)4,(long)1,(4)*sizeof(char),
ECPGt_NO_INDICATOR, NULL , 0L, 0L, 0L, ECPGt_EOIT,
ECPGt_descriptor, (desc1), 0L, 0L, 0L,
ECPGt_NO_INDICATOR, NULL , 0L, 0L, 0L, ECPGt_EORT);}
So, the above line is executed in ECPGdo(). On the other hand, desc1 is not
aligned on
8-byte boundary. This unaligned access causes SIGBUS.
[Fix]
Because desc1 is a char array, else block should be executed instead of the
above path.
var-value = var-pointer;
Therefore, make ecpg pass SQL descriptor host variables to ECPGdo() with
non-zero lengths.
Regards
MauMau
--
--
Zolta'n Boszorme'nyi
Cybertec Schonig Schonig GmbH
Grohrmuhlgasse 26
A-2700 Wiener Neustadt, Austria
Web: http://www.postgresql-support.de
http://www.postgresql.at/
Re: [HACKERS] INSERT...ON DUPLICATE KEY LOCK FOR UPDATE
On Tue, Dec 24, 2013 at 4:09 AM, Andres Freund and...@2ndquadrant.com wrote: I don't really see the lack of review as being crucial at this point. At least I have quite some doubts about the approach you've chosen and I have voiced it - so have others. Apparently you haven't been keeping up with this thread. The approach that Heikki outlined with his POC patch was demonstrated to deadlock in an unprincipled manner - it took me a relatively long time to figure this out because I didn't try a simple enough test-case. There is every reason to think that alternative promise tuple approaches would behave similarly, short of some very invasive, radical changes to how we wait on XID share locks that I really don't think are going to fly. That's why I chose this approach: at no point did anyone have a plausible alternative that didn't have similar problems, and I personally saw no alternative. It wasn't really a choice at all. In hindsight I should have known better than to think that people would be willing to defer discussion of a more acceptable value locking implementation to consider the interactions between the different subsystems, which I felt were critical and warranted up-front discussion, a belief which has now been borne out. Lesson learned. It's a pity that that's the way things are, because that discussion could have been really useful, and saved us all some time. I don't think there's too much reviewers can do before you've provided a POC implementation of real value locking. I don't see what is functionally insufficient about the value locking that you've already seen. I'm currently working towards extended the buffer locking to use a heavyweight lock held only for an instant, but potentially across multiple operations, although of course only when upserting occurs so as to not regress regular insertion. If you're still of the opinion that it is necessary to hold value locks of some form on earlier unique indexes, as you wait maybe for hours on some conflicting xid, then I still disagree with you for reasons recently re-stated [1]. You never stated a reason why you thought it was necessary. If you have one now, please share it. Note that I release all value locks before row locking too, which is necessary because to do any less will cause unprincipled deadlocking, as we've seen. Other than that, I have no idea what your continued objection to my design would be once the buffer level exclusive locks are replaced with page level heavyweight locks across complex (though brief) operations (I guess you might not like the visibility stuff or the syntax, but that isn't what you're talking about here). More granular value locking might help boost performance, but maybe not even by much, since we're only locking a single leaf page per unique index against insertion, and only for an instant. I see no reason to make the coarser-than-necessary granularity of the value locking a blocker. Predicate locks on btree leaf pages acquired by SSI are also coarser than strictly necessary. [1] http://www.postgresql.org/message-id/cam3swzsodumg4899tjc09r2uortyhb0vl9aasc1fz7aw4gs...@mail.gmail.com -- 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] [bug fix] ECPG app crashes due to SIGBUS on SPARC Solaris
From: Boszormenyi Zoltan z...@cybertec.at The app wasn't attached, only the patch. If this is a small test app, it can also be a part of the patch in the form of a regression test. Sorry, attached. Thank you for pointing it out. Regards MauMau sigbus.pgc 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] trailing comment ghost-timing
On 12/24/2013 08:53 AM, Martijn van Oosterhout wrote: (Untested). Isn't this just a case of psql not printing out a timing if the server responds with PGRES_EMPTY_QUERY? Yes, it is. Sorry should have made myself more clear (way more clear when I read my messages from yesterday). Then I thought the server should print timing even if PGRES_EMPTY_QUERY is returned. Now after thinking on it I am not so sure. -- Andreas Karlsson -- 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] Logging WAL when updating hintbit
On Wed, Dec 25, 2013 at 2:36 AM, Fujii Masao masao.fu...@gmail.com wrote: On Tue, Dec 24, 2013 at 1:58 PM, Michael Paquier michael.paqu...@gmail.com wrote: On Sat, Dec 21, 2013 at 4:13 AM, Robert Haas robertmh...@gmail.com wrote: On Fri, Dec 20, 2013 at 9:06 AM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Michael Paquier escribió: On Fri, Dec 20, 2013 at 1:05 PM, Sawada Masahiko sawada.m...@gmail.com wrote: Sorry the patch which I attached has wrong indent on pg_controldata. I have modified it and attached the new version patch. Now that you send this patch, I am just recalling some recent email from Tom arguing about avoiding to mix lower and upper-case characters for a GUC parameter name: http://www.postgresql.org/message-id/30569.1384917...@sss.pgh.pa.us To fullfill this requirement, could you replace walLogHints by wal_log_hints in your patch? Thoughts from others? The issue is with the user-visible variables, not with internal variables implementing them. I think the patch is sane. (Other than the fact that it was posted as a patch-on-patch instead of patch-on-master). But spelling it the same way everywhere really improves greppability. Yep, finding all the code paths with a single keyword is usually a good thing. Attached is a purely-aesthetical patch that updates the internal variable name to wal_log_hints. There are many GUC parameters other than wal_log_hints, that their names are not the same as the names of their variables. We should update also them? IMO, this looks hard to accept as some existing extensions would break (even if fix would be trivial) and it would make back-patching more difficult. wal_log_hints is a new parameter though... Regards, -- 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] Planning time in explain/explain analyze
On 12/24/2013 02:31 PM, Peter Eisentraut wrote: On 12/23/13, 8:12 PM, Andreas Karlsson wrote: A user asked in -performance[1] if there is a way to measure the planning time. log_planner_stats Thanks for the suggestion, I will use that if I need to know the plan times. But I do not think this is as useful as having it in the EXPLAIN output. -- Andreas Karlsson -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: [COMMITTERS] pgsql: pg_prewarm, a contrib module for prewarming relationd data.
On Fri, Dec 20, 2013 at 10:16 PM, Robert Haas rh...@postgresql.org wrote: pg_prewarm, a contrib module for prewarming relationd data. Patch by me. Review by Álvaro Herrera, Amit Kapila, Jeff Janes, Gurjeet Singh, and others. Branch -- master Details --- http://git.postgresql.org/pg/commitdiff/c32afe53c2e87a56e2ff930798a5588db0f7a516 Modified Files -- contrib/Makefile |1 + contrib/pg_prewarm/Makefile| 18 +++ contrib/pg_prewarm/pg_prewarm--1.0.sql | 14 +++ contrib/pg_prewarm/pg_prewarm.c| 205 contrib/pg_prewarm/pg_prewarm.control |5 + doc/src/sgml/contrib.sgml |1 + doc/src/sgml/filelist.sgml |1 + doc/src/sgml/pgprewarm.sgml| 68 +++ src/tools/pgindent/typedefs.list |1 + 9 files changed, 314 insertions(+) Hi Robert, It seems that you have forgotten pg_prewarm--unpackaged--1.0.sql when committing this extension. Patch attached fixes that. Thanks, -- Michael commit f2d0615e022a3079a69316adaae9c825e0d2 Author: Michael Paquier mich...@otacoo.com Date: Wed Dec 25 00:07:08 2013 +0900 Add unpackaged scripts for pg_prewarm It looks to have been forgotten when this patch has been committed. diff --git a/contrib/pg_prewarm/Makefile b/contrib/pg_prewarm/Makefile index 176a29a..85bdd9d 100644 --- a/contrib/pg_prewarm/Makefile +++ b/contrib/pg_prewarm/Makefile @@ -4,7 +4,7 @@ MODULE_big = pg_prewarm OBJS = pg_prewarm.o EXTENSION = pg_prewarm -DATA = pg_prewarm--1.0.sql +DATA = pg_prewarm--1.0.sql pg_prewarm--unpackaged--1.0.sql ifdef USE_PGXS PG_CONFIG = pg_config diff --git a/contrib/pg_prewarm/pg_prewarm--unpackaged--1.0.sql b/contrib/pg_prewarm/pg_prewarm--unpackaged--1.0.sql new file mode 100644 index 000..f40c2b8 --- /dev/null +++ b/contrib/pg_prewarm/pg_prewarm--unpackaged--1.0.sql @@ -0,0 +1,10 @@ +/* contrib/pg_prewarm/pg_prewarm--unpackaged--1.0.sql */ + +-- complain if script is sourced in psql, rather than via CREATE EXTENSION +\echo Use CREATE EXTENSION pg_prewarm to load this file. \quit + +ALTER EXTENSION pg_prewarm ADD function pg_prewarm_pages(regclass, + text, + text, + int8, + int8); -- 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] Planning time in explain/explain analyze
On 12/24/2013 05:09 AM, Robert Haas wrote: Yeah. The alternatives seem to be: 1. Change a lot of regression tests. This would be a serious PITA, not so much for the one-time cost as for every time we needed to back-patch a regression test that includes explain output. -1. 2. Don't display the planning time by default, necessitating a new PLANNING_TIME ON option. This has backwards compatibility to recommend it, but not much else. 3. Let COSTS OFF suppress it. Another option would be to not display it by default, but make VERBOSE show it. However, I think making it controlled by COSTS is a better fit. After some thinking COSTS OFF really seems to be the best option. I have changed this in the patch and will submit a patch with updated documentation as soon as I get the time. Adding Planning time to all explain outputs (only 37 of them) in the documentation should not clutter anything too much. -- Andreas Karlsson -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: [COMMITTERS] pgsql: pg_prewarm, a contrib module for prewarming relationd data.
Michael Paquier michael.paqu...@gmail.com writes: It seems that you have forgotten pg_prewarm--unpackaged--1.0.sql when committing this extension. Patch attached fixes that. Um, what's that needed for? There's no pre-extension version of this module to worry about upgrading from. Or did I miss something? 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
