Re: [HACKERS] Autonomous Transaction (WIP)
On 09 April 2014 21:25, Robert Haas Wrote: Deadlock Detection: I'm not sure how this would work out internally In order to resolve deadlock, two member variable will be created in the structure PROLOCK: Bitmask for lock types currently held by autonomous transaction. LOCKMASKholdMaskByAutoTx[MAX_AUTO_TX_LEVEL] Bitmask for lock types currently held by main transaction. LOCKMASKholdMaskByNormalTx Now when we grant the lock to particular transaction, depending on type of transaction, bit Mask will be set for either holdMaskByAutoTx or holdMaskByNormalTx. Similar when lock is ungranted, corresponding bitmask will be reset. That sounds pretty ugly, not to mention the fact that it will cause a substantial increase in the amount of memory required to store PROCLOCKs. It will probably slow things down, too. Actually I followed above design to keep it align with the existing design. As I understand, currently also all lock conflict is checked based on the corresponding lock bit mask. This is good catch that shared memory required will increase but isn't it justified from user perspective since we are allowing more transactions per session and hence memory required to store various kind of resources will increase. Since we are just additionally setting the bitmask for each lock (in-case there is autonomous transaction, then there will be one more additional bit mask setting and deadlock check), I don't think it should slow down the overall operation. Also We can keep number of autonomous transaction configurable(default-0), to keep it less impacting incase it is not configured. An autonomous transaction can also conflict with main transaction, so in order to check conflict between them, I am distinguishing at this level. Please correct me If I am wrong anywhere and also please provide your thought on this and on overall design. Thanks and Regards, Kumar Rajeev Rastogi -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] proposal: interprocess EXPLAIN PID
Hello I propose a enhancing of EXPLAIN statement about possibility get a plan of other PostgreSQL process. With some other enhancing this technique can be interesting for monitoring long duration queries. Notes, comments? Regards Pavel Stehule
Re: [HACKERS] Problem with displaying wide tables in psql
Hi.
I've done some corrections for printing newline and wrap indicators.
Please review the attached patch.
2014-04-11 0:14 GMT+04:00 Sergey Muraviov sergey.k.murav...@gmail.com:
Hi.
Thanks for your tests.
I've fixed problem with headers, but got new one with data.
I'll try to solve it tomorrow.
2014-04-10 18:45 GMT+04:00 Greg Stark st...@mit.edu:
Ok, So I've hacked on this a bit. Below is a test case showing the
problems I've found.
1) It isn't using the newline and wrap indicators or dividing lines.
2) The header is not being displayed properly when it contains a newline.
I can hack in the newline and wrap indicators but the header
formatting requires reworking the logic a bit. The header and data
need to be stepped through in parallel rather than having a loop to
handle the wrapping within the handling of a single line. I don't
really have time for that today but if you can get to it that would be
fine,
--
Best regards,
Sergey Muraviov
--
Best regards,
Sergey Muraviov
From 5e0f44994d04a81523920a78d3a35603e919170c Mon Sep 17 00:00:00 2001
From: Sergey Muraviov sergey.k.murav...@gmail.com
Date: Fri, 11 Apr 2014 11:03:41 +0400
Subject: [PATCH] Using newline and wrap indicators
---
src/bin/psql/print.c | 130 +++
1 file changed, 110 insertions(+), 20 deletions(-)
diff --git a/src/bin/psql/print.c b/src/bin/psql/print.c
index 79fc43e..6463162 100644
--- a/src/bin/psql/print.c
+++ b/src/bin/psql/print.c
@@ -1234,13 +1234,56 @@ print_aligned_vertical(const printTableContent *cont, FILE *fout)
fprintf(fout, %s\n, cont-title);
}
+ if (cont-opt-format == PRINT_WRAPPED)
+ {
+ 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++)
{
printTextRule pos;
- int line_count,
+ int dline,
+ hline,
dcomplete,
- hcomplete;
+ hcomplete,
+ offset,
+ chars_to_output;
if (cancel_pressed)
break;
@@ -1270,48 +1313,95 @@ print_aligned_vertical(const printTableContent *cont, FILE *fout)
pg_wcsformat((const unsigned char *) *ptr, strlen(*ptr), encoding,
dlineptr, dheight);
- line_count = 0;
+ dline = hline = 0;
dcomplete = hcomplete = 0;
+ offset = 0;
+ chars_to_output = dlineptr[dline].width;
while (!dcomplete || !hcomplete)
{
+ /* Left border */
if (opt_border == 2)
fprintf(fout, %s , dformat-leftvrule);
+
+ /* Header */
if (!hcomplete)
{
-fprintf(fout, %-s%*s, hlineptr[line_count].ptr,
- hwidth - hlineptr[line_count].width, );
+int swidth = hwidth - hlineptr[hline].width - 1;
+fprintf(fout, %-s, hlineptr[hline].ptr);
+if (swidth 0) /* spacer */
+ fprintf(fout, %*s, swidth, );
-if (!hlineptr[line_count + 1].ptr)
+if (!hlineptr[hline + 1].ptr)
+{
+ fputs( , fout);
hcomplete = 1;
+}
+else
+{
+ fputs(format-nl_right, fout);
+ hline++;
+}
}
else
-fprintf(fout, %*s, hwidth, );
+fprintf(fout, %*s, hwidth + 1, );
+ /* Separator */
if (opt_border 0)
-fprintf(fout, %s , dformat-midvrule);
- else
-fputc(' ', fout);
+fprintf(fout, %s, dformat-midvrule);
+ /* Data */
if (!dcomplete)
{
-if (opt_border 2)
- fprintf(fout, %s\n, dlineptr[line_count].ptr);
+int target_width,
+ bytes_to_output,
+ swidth;
+
+fputs(!dcomplete !offset? : format-wrap_left, fout);
+
+target_width = dwidth;
+bytes_to_output = strlen_max_width(dlineptr[dline].ptr + offset,
+ target_width, encoding);
+fputnbytes(fout, (char *)(dlineptr[dline].ptr + offset),
+ bytes_to_output);
+
+chars_to_output -= target_width;
+offset += bytes_to_output;
+
+/* spacer */
+swidth = dwidth - target_width;
+if (swidth 0)
+ fprintf(fout, %*s, swidth, );
+
+if (!chars_to_output)
+{
+ if (!dlineptr[dline + 1].ptr)
+ {
+ fputs( , fout);
+ dcomplete = 1;
+ }
+ else
+ {
+ fputs(format-nl_right, fout);
+ dline++;
+ offset = 0;
+ chars_to_output = dlineptr[dline].width;
+ }
+}
else
- fprintf(fout, %-s%*s %s\n, dlineptr[line_count].ptr,
- dwidth - dlineptr[line_count].width, ,
Re: [HACKERS] WIP patch (v2) for updatable security barrier views
On 11 April 2014 04:04, Stephen Frost sfr...@snowman.net wrote:
Dean, Craig, all,
* Dean Rasheed (dean.a.rash...@gmail.com) wrote:
This is reflected in the change to the regression test output where,
in one of the tests, the ctids for the table to update are no longer
coming from the same table. I think a better approach is to push down
the rowmark into the subquery so that any locking required applies to
the pushed down RTE --- see the attached patch.
I'm working through this patch and came across a few places where I
wanted to ask questions (as much for my own edification as questioning
the actual implementation). Also, feel free to point out if I'm
bringing up something which has already been discussed. I've been
trying to follow the discussion but it's been a while and my memory may
have faded.
Thanks for looking at this.
While in the planner, we need to address the case of a child RTE which
has been transformed from a relation to a subquery. That all makes
perfect sense, but I'm wondering if it'd be better to change this
conditional:
! if (rte1-rtekind == RTE_RELATION
! rte1-securityQuals != NIL
! rte2-rtekind == RTE_SUBQUERY)
which essentially says if a relation was changed to a subquery *and*
it has security quals then we need to update the entry into one like
this:
! if (rte1-rtekind == RTE_RELATION
! rte2-rtekind == RTE_SUBQUERY)
! {
! Assert(rte1-securityQuals != NIL);
! ...
which changes it to if a relation was changed to a subquery, it had
better be because it's got securityQuals that we're dealing with. My
general thinking here is that we'd be better off with the Assert()
firing during some later development which changes things in this area
than skipping the change because there aren't any securityQuals and then
expecting everything to be fine with the subquery actually being a
relation..
Yes, I think that makes sense, and it seems like a sensible check.
I could see flipping that around too, to check if there are
securityQuals and then Assert() on the change from relation to subquery-
after all, if there are securityQuals then it *must* be a subquery,
right?
No, because a relation with securityQuals is only changed to a
subquery if it is actually used in the main query (see the check in
expand_security_quals). During the normal course of events when
working with an inheritance hierarchy, there are RTEs for other
children that aren't referred to in the main query for the current
inheritance child, and those RTEs are not expanded into subqueries.
A similar case exists in prepunion.c where we're checking if we should
recurse while in adjust_appendrel_attrs_mutator()- the check exists as
! if (IsA(node, Query))
(... which used to be an Assert(!IsA(node, Query)) ...)
but the comment is then quite clear that we should only be doing this in
the security-barrier case; perhaps we should Assert() there to that
effect? It'd certainly make me feel a bit better about removing the two
Asserts() which were there; presumably we had to also remove the
Assert(!IsA(node, SubLink)) ?
When I originally wrote those comments, I thought that this change
would only apply to securityQuals. Later, following a seemingly
unrelated bug involving UPDATEs containing LATERAL references to the
target relation (which is now disallowed), I thought that this change
might help with that case too, if such UPDATEs were to be allowed
again in the future
(http://www.postgresql.org/message-id/CAEZATCXpOsF5wZ1XXWQur7G5M52=mwzuaqye9b0rgqhxvw3...@mail.gmail.com).
For now though, the comments are correct, and it can only happen with
securityQuals. Adding an Assert() to that effect might be a bit fiddly
though, because the information required isn't available on the local
Node being processed, so I think it would have to add and maintain an
additional flag on the mutator context. I'm not sure that it's worth
it.
Also, it seems like there should be a check_stack_depth() call here now?
Hm, perhaps. I don't see any such checks in the rewriter though, and I
wouldn't expect the call depth here to get any deeper than it had
previously done there.
That covers more-or-less everything outside of prepsecurity.c itself.
I'm planning to review that tomorrow night. In general, I'm pretty
happy with the shape of this. The wiki and earlier discussions were
quite useful. My one complaint about this is that it feels like a few
more comments and more documentation updates would be warrented; and in
particular we need to make note of the locking gotcha in the docs.
That's not a solution, of course, but since we know about it we should
probably make sure users are aware.
Am I right in thinking that the locking gotcha only happens if you
create a security_barrier view conaining a SELECT ... FOR UPDATE? If
so, that seems like rather a
Re: [HACKERS] Using indices for UNION.
Hello. Thank you for the attentive comments. I wrote: I still think this stuff mostly needs to be thrown away and rewritten in Path-creation style, but that's a long-term project. In the meantime this seems like a relatively small increment of complexity, so maybe it's worth applying. I'm concerned about the method for building a new DISTINCT clause though; the best that can be said for that is that it's a crude hack, and I'm less than convinced that there are no cases where it'll dump core. OK, after still more time thinking about it, here's the issues with that way of generating a DISTINCT clause corresponding to the UNION: 1. Calling transformDistinctClause with a NULL pstate seems pretty unsafe. It might accidentally fail to fail, today, but it's surely fragile. It's violating the API contract not only of transformDistinctClause itself (which is not documented as accepting a NULL pstate) but of addTargetToGroupList (q.v.). Hmm I'm ashamed to have missed addTargetToGroupList. You are right about that. I saw only parser_errposition to field the NULL.. It should be fixed anyway. A minimum requirement before I'd accept a patch that did that is that it extended the header comments of those functions to specify under what circumstances a NULL pstate can be passed. However, that's not the direction to go, because of #2. 2. This approach potentially changes the semantics of the UNION. (This is only important for a datatype that has more than one btree equality operator, but let's posit that.) transformDistinctClause, as per its header comment, allows the outer ORDER BY to influence which equality operator it picks for DISTINCT. However, in the case of (SELECT ... UNION SELECT ...) ORDER BY ..., the UNION semantics were chosen without reference to the ORDER BY, If my understanding is correct, the query is equivalent to it without the parens. The ORDER BY belongs to the topmost UNION on both cases. Actually planner compailns about (SELECT...UNION SELECT ... ORDER BY) ORDER BY that multiple ORDER BY clauses not allowed with/without this patch. so it's possible that the equality operators cited in the SetOperationStmt's groupClauses list are not what you'd get from applying transformDistinctClause as the patch does. This patch flattenes and the SetOperationStmt was put out along with its groupClause. But the groupClauses is originally generated from targetlist in transformSetOperationTree. And the new DistinctClause is generated also from the same targetlist consulting to sortClause. They are not in the same order but it doesn't matter. Is it wrong? If it would make some trouble, I could add an check it out or count it on making the DistinctClauses.. Finally, explain (costs off) select a, b from c11 union select a, b from c12 order by b limit 1; |QUERY PLAN | - | Limit |- Sort | Sort Key: c11.b | - HashAggregate |Group Key: c11.b, c11.a |- Append | - Seq Scan on c11 | - Seq Scan on c12 This HashAggregate does DISTINCT which was performed by using Sort-Unique without this patch, and yields the same result, I believe. It is not okay for the planner to override the parser's choice of semantics like that. As described above, as far as I saw, itself seems to be a problem. Now I'm fairly sure that planner.c is expecting that the query's distinctClause be a superset of the sortClause if any (cf comments for SortGroupClause in parsenodes.h), so it wouldn't be okay to just blindly build a distinctClause from topop-groupClauses. Yes, it could be but counting the sortClause into distinctClause is crucial factor for getting rid of unnecessary sorting. I think what you need to do is check that topop-groupClauses is compatible with the sortClause if any (skipping the flattening transformation if not) and then build a distinctClause by extending the sort clause list with any missing items from topop-groupClauses. Ah, yes I agree as I described above. So this is sort of like what transformDistinctClause does, but different in detail, and the failure case is to not do the transformation, rather than ereport'ing. (See also generate_setop_grouplist, which you could almost use except that it's not expecting to have to merge with a sortClause list.) Thank you. I'll follow this advise. So this is all doable enough, but you're probably going to end up with a new distinctClause-generating function that's at least twice the size of the patch's former net code addition. So the question is whether it's worth the trouble, considering that all this code has (I hope) a life expectancy of no more than one or two more release cycles. Hmm. Since this is a kind of local optimization, some future drastic reconstructing might make this useless. But it doesn't seem the reason not
Re: [HACKERS] [PATCH] Negative Transition Aggregate Functions (WIP)
On 10 April 2014 22:52, Tom Lane t...@sss.pgh.pa.us wrote: Dean Rasheed dean.a.rash...@gmail.com writes: I was imagining that firsttrans would only be passed the first value to be aggregated, not any previous state, and that it would be illegal to specify both an initcond and a firsttrans function. The forward transition function would only be called for values after the first, by which point the state would be non-null, and so it could be made strict in most cases. The same would apply to the invertible transition functions, so they wouldn't have to do null counting, which in turn would make their state types simpler. I put together a very fast proof-of-concept patch for this (attached). It has a valid execution path for an aggregate with initfunc, but I didn't bother writing the CREATE AGGREGATE support yet. I made sum(int4) work as you suggest, marking the transfn strict and ripping out int4_sum's internal support for null inputs. The result seems to be about a 4% or so improvement in the overall aggregation speed, for a simple SELECT sum(int4col) FROM table query. So from a performance standpoint this seems only marginally worth doing. The real problem is not that 4% isn't worth the trouble, it's that AFAICS the only built-in aggregates that can benefit are sum(int2) and sum(int4). So that looks like a rather narrow use-case. You had suggested upthread that we could use this idea to make the transition functions strict for aggregates using internal transition datatypes, but that does not work because the initfunc would violate the safety rule that a function returning internal must take at least one internal-type argument. That puts a pretty strong damper on the usefulness of the approach, given how many internal-transtype aggregates we have (and the moving-aggregate case is not going to be better is it?) Ah, that's disappointing. If it can't be made to work for aggregates with internal state types, then I think it loses most of it's value, and I don't think it will be of much more use in the moving aggregate case either. So at this point I'm feeling unexcited about the initfunc idea. Unless it does something really good for the moving-aggregate case, I think we should drop it. Agreed. Thanks for prototyping it though. Regards, Dean -- 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] Minor improvements in alter_table.sgml
(2014/04/09 12:03), Etsuro Fujita wrote: (2014/04/09 1:23), Robert Haas wrote: On Tue, Apr 8, 2014 at 5:05 AM, Etsuro Fujita fujita.ets...@lab.ntt.co.jp wrote: Attached is a patch to improve the manual page for the ALTER TABLE command. Do we really need to add a section for type_name when we already have a section for OF type_name? I think that the section for type_name would be necessary as that in chapter Parameters, not in chapter Description, which includes the section for OF type_name. constraint_name is also used for adding a constraint using an index. So it could not only be a constraint to alter, validate, or drop, but also a new constraint name to be added. I overlooked that. Honestly, how much value is there in even having a section for this? Do we really want to document constraint_name as name of an existing constraint, or the name of a new constraint to be added? It would be accurate, then, but it also doesn't really tell you anything you didn't know already. You have a point there, but I feel odd about the documentation as is, because some are well written (eg, column_name) and some are not (eg, constraint_name). So, if there are no objections, I'd like to update the patch. Attached is an updated version of the patch. Thanks, Best regards, Etsuro Fujita diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index 0b08f83..c16fc19 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -582,8 +582,8 @@ ALTER TABLE [ IF EXISTS ] replaceable class=PARAMETERname/replaceable termliteralOWNER/literal/term listitem para - This form changes the owner of the table, sequence, or view to the - specified user. + This form changes the owner of the table, sequence, view, materialized view, + or foreign table to the specified user. /para /listitem /varlistentry @@ -625,8 +625,9 @@ ALTER TABLE [ IF EXISTS ] replaceable class=PARAMETERname/replaceable listitem para The literalRENAME/literal forms change the name of a table - (or an index, sequence, or view), the name of an individual column in - a table, or the name of a constraint of the table. There is no effect on the stored data. + (or an index, sequence, view, materialized view, or foreign table), the name + of an individual column in a table, or the name of a constraint of the table. + There is no effect on the stored data. /para /listitem /varlistentry @@ -708,38 +709,47 @@ ALTER TABLE [ IF EXISTS ] replaceable class=PARAMETERname/replaceable /varlistentry varlistentry - termreplaceable class=PARAMETERnew_name/replaceable/term + termreplaceable class=PARAMETERconstraint_name/replaceable/term listitem para -New name for the table. +Name of a new or existing constraint. /para /listitem /varlistentry varlistentry - termreplaceable class=PARAMETERtype/replaceable/term + termreplaceable class=PARAMETERnew_constraint_name/replaceable/term listitem para -Data type of the new column, or new data type for an existing -column. +New name for an existing constraint. /para /listitem /varlistentry varlistentry - termreplaceable class=PARAMETERtable_constraint/replaceable/term + termreplaceable class=PARAMETERnew_name/replaceable/term listitem para -New table constraint for the table. +New name for the table. /para /listitem /varlistentry varlistentry - termreplaceable class=PARAMETERconstraint_name/replaceable/term + termreplaceable class=PARAMETERdata_type/replaceable/term listitem para -Name of an existing constraint to drop. +Data type of the new column, or new data type for an existing +column. + /para + /listitem + /varlistentry + + varlistentry + termreplaceable class=PARAMETERtable_constraint/replaceable/term + listitem + para +New table constraint for the table. /para /listitem /varlistentry @@ -799,6 +809,15 @@ ALTER TABLE [ IF EXISTS ] replaceable class=PARAMETERname/replaceable /varlistentry varlistentry + termreplaceable class=PARAMETERrewrite_rule_name/replaceable/term + listitem + para +Name of a single rewrite rule to disable or enable. + /para + /listitem + /varlistentry + + varlistentry termreplaceable class=PARAMETERindex_name/replaceable/term listitem para @@ -836,6 +855,15 @@ ALTER TABLE [ IF EXISTS ] replaceable class=PARAMETERname/replaceable /varlistentry varlistentry + termreplaceable class=PARAMETERtype_name/replaceable/term + listitem + para +The name of a composite type. +
Re: [HACKERS] WIP patch (v2) for updatable security barrier views
Dean, * Dean Rasheed (dean.a.rash...@gmail.com) wrote: On 11 April 2014 04:04, Stephen Frost sfr...@snowman.net wrote: which changes it to if a relation was changed to a subquery, it had better be because it's got securityQuals that we're dealing with. My general thinking here is that we'd be better off with the Assert() firing during some later development which changes things in this area than skipping the change because there aren't any securityQuals and then expecting everything to be fine with the subquery actually being a relation.. Yes, I think that makes sense, and it seems like a sensible check. Ok, I'll change that. I could see flipping that around too, to check if there are securityQuals and then Assert() on the change from relation to subquery- after all, if there are securityQuals then it *must* be a subquery, right? No, because a relation with securityQuals is only changed to a subquery if it is actually used in the main query (see the check in expand_security_quals). During the normal course of events when working with an inheritance hierarchy, there are RTEs for other children that aren't referred to in the main query for the current inheritance child, and those RTEs are not expanded into subqueries. Ah, yes, makes sense. For now though, the comments are correct, and it can only happen with securityQuals. Adding an Assert() to that effect might be a bit fiddly though, because the information required isn't available on the local Node being processed, so I think it would have to add and maintain an additional flag on the mutator context. I'm not sure that it's worth it. Yeah, I was afraid that might be the case. No problem. Also, it seems like there should be a check_stack_depth() call here now? Hm, perhaps. I don't see any such checks in the rewriter though, and I wouldn't expect the call depth here to get any deeper than it had previously done there. Hmm, ok. I'll think on that one. That covers more-or-less everything outside of prepsecurity.c itself. I'm planning to review that tomorrow night. In general, I'm pretty happy with the shape of this. The wiki and earlier discussions were quite useful. My one complaint about this is that it feels like a few more comments and more documentation updates would be warrented; and in particular we need to make note of the locking gotcha in the docs. That's not a solution, of course, but since we know about it we should probably make sure users are aware. Am I right in thinking that the locking gotcha only happens if you create a security_barrier view conaining a SELECT ... FOR UPDATE? If so, that seems like rather a niche case - not that that means we shouldn't warn people about it. Hmm, the 'gotcha' I was referring to was the issue discussed upthread around rows getting locked to be updated which didn't pass all the quals (they passed the security_barrier view's, but not the user-supplied ones), which could happen during a normal 'update' against a security_barrier view, right? I didn't think that would require the view definition to be 'FOR UPDATE'; if that's required then it would seem like we're actually doing what the user expects based on their view definition.. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] PostgreSQL in Windows console and Ctrl-C
On 04/11/2014 01:35 AM, Amit Kapila wrote:
On Fri, Apr 11, 2014 at 3:14 AM, Bruce Momjian br...@momjian.us wrote:
Can someone with Windows expertise comment on whether this should be
applied?
I don't think this is a complete fix, for example what about platform where
_CreateRestrictedToken() is not supported. For Example, the current
proposed fix will not work for below case:
if (_CreateRestrictedToken == NULL)
{
/*
* NT4 doesn't have CreateRestrictedToken, so just call ordinary
* CreateProcess
*/
Are we really supporting NT4 any more? Even XP is about to be at end of
support from Microsoft.
cheers
andrew
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PostgreSQL in Windows console and Ctrl-C
On 2014-04-11 07:12:50 -0400, Andrew Dunstan wrote:
On 04/11/2014 01:35 AM, Amit Kapila wrote:
On Fri, Apr 11, 2014 at 3:14 AM, Bruce Momjian br...@momjian.us wrote:
Can someone with Windows expertise comment on whether this should be
applied?
I don't think this is a complete fix, for example what about platform where
_CreateRestrictedToken() is not supported. For Example, the current
proposed fix will not work for below case:
if (_CreateRestrictedToken == NULL)
{
/*
* NT4 doesn't have CreateRestrictedToken, so just call ordinary
* CreateProcess
*/
Are we really supporting NT4 any more? Even XP is about to be at end of
support from Microsoft.
I seem to recall that win2k was already desupported?
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] WIP patch (v2) for updatable security barrier views
(Sorry if this breaks the thread history; on mobile) Am I right in thinking that the locking gotcha only happens if you create a security_barrier view conaining a SELECT ... FOR UPDATE? If so, that seems like rather a niche case - not that that means we shouldn't warn people about it. Hmm, the 'gotcha' I was referring to was the issue discussed upthread around rows getting locked to be updated which didn't pass all the quals (they passed the security_barrier view's, but not the user-supplied ones), which could happen during a normal 'update' against a security_barrier view, right? I didn't think that would require the view definition to be 'FOR UPDATE'; It doesn't require the view to be defined FOR UPDATE. I'll try to write an isolstiontester case to donstrate this on the weekend. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] [feature] cached index to speed up specific queries on extremely large data sets
hi folks, please cc me direct on responses as i am subscribed on digest. i've been asked to look at how to deal with around 7 billion records (appx 30 columns, appx data size total 1k) and this might have to be in a single system (i will need to Have Words with the client about that). the data is read-only and an arbitrary number of additional tables may be created to manage the data. records come in at a rate of around 25 million per day, the 7 billion records is based on the assumption of keeping one month's worth of data around. analysis of this data needs to be done across the entire set: i.e. it may not be subdivided into isolated tables (by day for example). i am therefore um rather concerned about efficiency, even just from the perspective of using 2nd normalised form and not doing JOINs against other tables. so i had an idea. there already exists the concept of indexes. there already exists the concept of cached queries. question: would it be practical to *merge* those two concepts such that specific queries could be *updated* as new records are added, such that when the query is called again it answers basically pretty much immediately? let us assume that performance degradation on update (given that indexes already exist and are required to be updated) is acceptable. the only practical way (without digging into postgresql's c code) to do this at a higher level would be in effect to abandon the advantages of the postgresql query optimisation engine and *reimplement* it in a high-level language, subdividing the data into smaller (more manageable) tables, using yet more tables to store intermediate results of a previous query then somehow managing to stitch together a new response based on newer packets. it would be a complete nightmare to both implement and maintain. second question then based on whether the first is practical: is there anyone who would be willing (assuming it can be arranged) to engage in a contract to implement the required functionality? thanks, l. -- 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] [feature] cached index to speed up specific queries on extremely large data sets
On 04/11/2014 03:20 PM, lkcl . wrote: so i had an idea. there already exists the concept of indexes. there already exists the concept of cached queries. question: would it be practical to*merge* those two concepts such that specific queries could be*updated* as new records are added, such that when the query is called again it answers basically pretty much immediately? let us assume that performance degradation on update (given that indexes already exist and are required to be updated) is acceptable. I think you just described materialized views. The built-in materialized views in PostgreSQL are not updated immediately as the tables are modified, but it's entirely possible to roll your own using views and triggers. There are a few links on the PostgreSQL wiki, in the Versions before 9.3 section: https://wiki.postgresql.org/wiki/Materialized_Views. - Heikki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP patch (v2) for updatable security barrier views
* Craig Ringer (cr...@2ndquadrant.com) wrote: Hmm, the 'gotcha' I was referring to was the issue discussed upthread around rows getting locked to be updated which didn't pass all the quals (they passed the security_barrier view's, but not the user-supplied ones), which could happen during a normal 'update' against a security_barrier view, right? I didn't think that would require the view definition to be 'FOR UPDATE'; It doesn't require the view to be defined FOR UPDATE. Ok, great, glad I got that correct. :) I'll try to write an isolstiontester case to donstrate this on the weekend. Great, thanks. I'll take a stab at writing up the 'gotcha' note tonight or tomorrow. Thanks again, Stephen signature.asc Description: Digital signature
Re: [HACKERS] [BUG FIX] Compare returned value by socket() against PGINVALID_SOCKET instead of 0
On Fri, Apr 11, 2014 at 10:03:08AM +0530, Amit Kapila wrote:
On Fri, Apr 11, 2014 at 10:00 AM, Amit Kapila amit.kapil...@gmail.com wrote:
On Thu, Apr 10, 2014 at 5:21 PM, Bruce Momjian br...@momjian.us wrote:
On Thu, Apr 10, 2014 at 11:05:49AM +0530, Amit Kapila wrote:
Ah, yes, good point. This is going to require backpatching then.
I also think so.
I think it's better to use check like below, just for matter of
consistency with other place
if (sock == INVALID_SOCKET)
Agreed. That is how I have coded the patch.
Sorry, I didn't checked the latest patch before that comment.
I verified that your last patch is fine. Regression test also went fine.
I have noticed small thing which I forgot to mention in previous mail.
I think below added extra line is not required.
int
PQsocket(const PGconn *conn)
{
+
Yes, I saw that yesterday and fixed it. I also did a dry run of
backpatching and only 8.4 had conflicts, so I think we are good there.
(This is like the readdir() fix all over again.)
Once this is applied I will work on changing the libpq socket type to
use portable pgsocket, but I am not planning to backpatch that unless we
find a bug.
--
Bruce Momjian br...@momjian.ushttp://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
--
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] [feature] cached index to speed up specific queries on extremely large data sets
On Fri, Apr 11, 2014 at 1:26 PM, Heikki Linnakangas hlinnakan...@vmware.com wrote: On 04/11/2014 03:20 PM, lkcl . wrote: so i had an idea. there already exists the concept of indexes. there already exists the concept of cached queries. question: would it be practical to*merge* those two concepts such that specific queries could be*updated* as new records are added, such that when the query is called again it answers basically pretty much immediately? let us assume that performance degradation on update (given that indexes already exist and are required to be updated) is acceptable. I think you just described materialized views. ... well... dang :) http://tech.jonathangardner.net/wiki/PostgreSQL/Materialized_Views ok so definitely not the snapshot materialised views, but yes! the eager materialised views, definitely. The built-in materialized views in PostgreSQL are not updated immediately as the tables are modified, ... but that would probably be enough. but it's entirely possible to roll your own using views and triggers. There are a few links on the PostgreSQL wiki, in the Versions before 9.3 section: https://wiki.postgresql.org/wiki/Materialized_Views. awesome. uhhh, well that was easy *lol*. once i am paid, whom do i send the payment to for the fast response and incredibly valuable information? :) [this is a serious question!] l. -- 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] [feature] cached index to speed up specific queries on extremely large data sets
On Fri, Apr 11, 2014 at 9:53 PM, lkcl . luke.leigh...@gmail.com wrote: On Fri, Apr 11, 2014 at 1:26 PM, Heikki Linnakangas hlinnakan...@vmware.com wrote: On 04/11/2014 03:20 PM, lkcl . wrote: so i had an idea. there already exists the concept of indexes. there already exists the concept of cached queries. question: would it be practical to*merge* those two concepts such that specific queries could be*updated* as new records are added, such that when the query is called again it answers basically pretty much immediately? let us assume that performance degradation on update (given that indexes already exist and are required to be updated) is acceptable. I think you just described materialized views. ... well... dang :) http://tech.jonathangardner.net/wiki/PostgreSQL/Materialized_Views ok so definitely not the snapshot materialised views, but yes! the eager materialised views, definitely. The built-in materialized views in PostgreSQL are not updated immediately as the tables are modified, ... but that would probably be enough. but it's entirely possible to roll your own using views and triggers. There are a few links on the PostgreSQL wiki, in the Versions before 9.3 section: https://wiki.postgresql.org/wiki/Materialized_Views. When updating a materialized view, or refreshing it, you need as well to be aware that an exclusive lock is taken on it during the refresh in 9.3, so the materialized view cannot be accessed for read queries. awesome. uhhh, well that was easy *lol*. once i am paid, whom do i send the payment to for the fast response and incredibly valuable information? :) [this is a serious question!] This can be helpful: http://www.postgresql.org/about/donate/ -- 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] [feature] cached index to speed up specific queries on extremely large data sets
On Fri, Apr 11, 2014 at 2:12 PM, Michael Paquier michael.paqu...@gmail.com wrote: On Fri, Apr 11, 2014 at 9:53 PM, lkcl . luke.leigh...@gmail.com wrote: section: https://wiki.postgresql.org/wiki/Materialized_Views. When updating a materialized view, or refreshing it, you need as well to be aware that an exclusive lock is taken on it during the refresh in 9.3, so the materialized view cannot be accessed for read queries. ok. as long as the storage of data (in the underlying table) is not adversely affected, that would be fine. as this is the hackers list and this turns out to be more a user question i'll leave it for now. awesome. uhhh, well that was easy *lol*. once i am paid, whom do i send the payment to for the fast response and incredibly valuable information? :) [this is a serious question!] This can be helpful: http://www.postgresql.org/about/donate/ thank you 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] WIP patch (v2) for updatable security barrier views
Stephen Frost sfr...@snowman.net writes: * Dean Rasheed (dean.a.rash...@gmail.com) wrote: Am I right in thinking that the locking gotcha only happens if you create a security_barrier view conaining a SELECT ... FOR UPDATE? If so, that seems like rather a niche case - not that that means we shouldn't warn people about it. Hmm, the 'gotcha' I was referring to was the issue discussed upthread around rows getting locked to be updated which didn't pass all the quals (they passed the security_barrier view's, but not the user-supplied ones), which could happen during a normal 'update' against a security_barrier view, right? I didn't think that would require the view definition to be 'FOR UPDATE'; if that's required then it would seem like we're actually doing what the user expects based on their view definition.. Yeah, the point of the gotcha is that a FOR UPDATE specified *outside* a security-barrier view would act as though it had appeared *inside* the view, since it effectively gets pushed down even though outer quals don't. 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
[HACKERS] Signaling of waiting for a cleanup lock?
Hi, VACUUM sometimes waits synchronously for a cleanup lock on a heap page. Sometimes for a long time. Without reporting it externally. Rather confusing ;). Since we only take cleanup locks around vacuum, how about we report at least in pgstat that we're waiting? At the moment, there's really no way to know if that's what's happening. 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] [PATCH] Negative Transition Aggregate Functions (WIP)
Florian Pflug f...@phlo.org writes: Which is why I feel that having two separate sets of transition functions and state types solves the wrong problem. If we find a way to prevent transition functions from being called directly, we'll shave a few cycles of quite a few existing aggregates, invertible or not. If we find a way around the initfunc-for-internal-statetype problem you discovered, the implementation would get much simpler, since we could then make nearly all of them strict. And this would again shave off a few cycles - for lots of NULL inputs, the effect could even be large. Since neither of those latter things seems likely to happen, I don't find this argument convincing at all. Even if they did happen, they would represent an incremental improvement that would be equally useful with either one or two sfuncs. Basically, this comes down to a design judgment call, and my judgment is differing from yours. In the absence of opinions from others, I'm going to do it my way. However, quite independently of how many sfuncs there are, I'm interested about this: ... SUM(int4) wouldn't need *any* extra state at all to be invertible, if it weren't for those pesky issues surrounding NULL handling. In fact, an earlier version of the invtrans patch *did* use int4_sum as SUM(int4)'s transfer functions! The only reason this doesn't work nowadays is that Dean didn't like the forward-nonstrict-but-inverse-strict special case that made this work. Tell me about the special case here again? Should we revisit the issue? 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] Problem with displaying wide tables in psql
Looks good.
It's still not doing the old-ascii column dividers but to be honest
I'm not sure what the intended behaviour of old-ascii is. I've noticed
old-ascii only displays the line markers for dividing lines, not the
final one. That seems pretty useless and maybe it's why we switched to
the new ascii mode, I don't recall.
This is the way old-ascii displays when two columns are wrapping -- it
seems pretty confused to me. The headers are displaying newline
markers from the ascii style instead of : indicators in the dividing
line. The : and ; markers are only displayed for the first column, not
the second one.
Maybe since the : and ; markers aren't shown for the final column that
means the expanded mode doesn't have to do anything since the column
is only ever the final column.
++-+
| x |x|
|+y |+ y|
|+z |+ z|
++-+
| x | xxx |
| xx ; |
| xxx: xxx |
| ; xxx |
| x : xxx |
| xx ; xx |
| xxx: xxx |
| ; x |
| x : xxx |
| xx : xx |
| xxx: x |
| : |
| x : xxx |
| xx : xx |
| xxx: x |
| : |
| x : xxx |
| xx : xx |
| xx : x |
| x : |
| xx : xxx |
| xx : xx |
| xx : x |
| xxx: |
| xx : |
| : |
| xx : |
| x : |
| xx : |
| xx |
| xx |
| xxx |
++-+
stark=# select array_to_string(array_agg(repeat('x',x)),E'\n') as x
y
z
from generate_series(1,20) as x(x);
┌─[ RECORD 1 ]─┐
│ x↵│ x ↵│
│ y↵│ xx ↵│
│ z │ xxx ↵│
│ │ ↵│
│ │ x ↵│
│ │ xx ↵│
│ │ xxx ↵│
│ │ ↵│
│ │ x ↵│
│ │ xx ↵│
│ │ xxx ↵│
│ │ ↵│
│ │ …│
│ │…x ↵│
│ │ …│
│ │…xx ↵│
│ │ …│
│ │…xxx ↵│
│ │ …│
│ │…↵│
│ │ …│
│ │…x ↵│
│ │ …│
│ │…xx ↵│
│ │ …│
│ │…xxx ↵│
│ │ …│
│ │… │
└───┴──┘
stark=# \pset linestyle ascii
Line style (linestyle) is ascii.
stark=# select array_to_string(array_agg(repeat('x',x)),E'\n') as x
y
z
from generate_series(1,20) as x(x);
stark***# stark***# stark-***# +-[ RECORD 1 ]-+
| x+| x +|
| y+| xx +|
| z | xxx +|
| | +|
| | x +|
| | xx +|
| | xxx +|
| | +|
| | x +|
| | xx +|
| | xxx +|
| | +|
| | .|
| |.x +|
| | .|
| |.xx +|
| | .|
| |.xxx +|
| | .|
| |.+|
| | .|
| |.x +|
| | .|
| |.xx +|
| | .|
| |.xxx +|
| | .|
| |. |
+---+--+
stark=# \pset linestyle old-ascii
Line style (linestyle) is old-ascii.
stark=# select array_to_string(array_agg(repeat('x',x)),E'\n') as x
y
z
from generate_series(1,20) as x(x);
stark# stark# stark-#
+-[ RECORD 1 ]-+
| x | x|
| y | xx |
| z | xxx |
| | |
| | x|
| | xx |
| | xxx |
| | |
| | x|
| | xx |
| | xxx |
| | |
| | |
| | x|
| | |
| | xx |
| | |
| | xxx |
| | |
| | |
| | |
| | x|
| | |
| | xx |
| | |
| | xxx |
| | |
| | |
+---+--+
stark=# \pset expanded off
Expanded display (expanded) is off.
stark=# \pset columns 40
Target width (columns) is 40.
stark=# select array_to_string(array_agg(repeat('x',x)),E'\n') as x
y
Re: [HACKERS] [PATCH] Negative Transition Aggregate Functions (WIP)
On Apr11, 2014, at 17:09 , Tom Lane t...@sss.pgh.pa.us wrote: Basically, this comes down to a design judgment call, and my judgment is differing from yours. In the absence of opinions from others, I'm going to do it my way. Ok. Are you going to do the necessary changes, or shall I? I'm happy to leave it to you, but if you lack the time I can try to find some. ... SUM(int4) wouldn't need *any* extra state at all to be invertible, if it weren't for those pesky issues surrounding NULL handling. In fact, an earlier version of the invtrans patch *did* use int4_sum as SUM(int4)'s transfer functions! The only reason this doesn't work nowadays is that Dean didn't like the forward-nonstrict-but-inverse-strict special case that made this work. Tell me about the special case here again? Should we revisit the issue? My original coding allows the combination of non-strict forward with strict reverse transition functions. The combination behaved like a strict aggregate regarding NULL handling - i.e., neither the forward nor the reverse transition function received NULL inputs. But if the initial state was NULL, the forward transition function *would* be called with that NULL state value upon seeing the first non-NULL input. In the window case, the aggregation machinery would also take care to reset the state type to it's initial value when it removed the last non-NULL input from the aggregation state (just like it does for strict aggregates today). This had two advantages 1) First, it allows strict aggregates to use state type internal. As it stands now, aggregates with state type internal must implement their own NULL handling, even if they behave exactly like most standard aggregates do, namely ignore NULLS and return NULL only if there were no non-NULL inputs. 2) It allows strict aggregates whose state type and input type aren't binary coercible to return NULL if all inputs were NULL without any special coding. As it stands today, this doesn't work without some kind of counter in the state, because the final function otherwise won't know if there were non-NULL inputs or not. Aggregates whose state and input types are binary coercible get around that by setting the initial value to NULL while *still* being strict, and relying on the state replacement behaviour of the aggregate machinery. It, however, also has a few disadvantages 3) It means that one needs to look at the inverse transition function's strictness setting even if that function is never used. 4) It feels a bit hacky. (2) is what affects SUM(int4). The only reason it track the number of inputs is to be able to return NULL instead of 0 if no inputs remain. One idea to fix (3) and (4) was *explicitly* flagging aggregates as NULL-handling or NULL-ignoring, and also as what one might call weakly strict, i.e. as returning NULL exactly if there were no non-NULL inputs. There are various variations of that theme possible - one flag for each behaviour, or simply a single common behaviour flag. In the end, I decided not to pursue that, mostly because the aggregates affected by that issued turned out to be relatively easy to fix. For the ones with state type internal, I simply added a counter, and I made SUM(int4) use AVG's transition function. I don't feel too strongly either way on this one - I initially implemented the exception because I noticed that the NULL handling of some aggregates was broken otherwise, and it seemed simpler to fix this in one place than going over all the aggregates separately. OTOH, when I wrote the docs, I noticed how hard it was to describe the behaviour accurately, which made me like it less and less. And Dean wasn't happy with it at all, so that finally settled it. best regards, Florian Pflug -- 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
I hope that I realized old-ascii logic correctly.
2014-04-11 19:10 GMT+04:00 Greg Stark st...@mit.edu:
Looks good.
It's still not doing the old-ascii column dividers but to be honest
I'm not sure what the intended behaviour of old-ascii is. I've noticed
old-ascii only displays the line markers for dividing lines, not the
final one. That seems pretty useless and maybe it's why we switched to
the new ascii mode, I don't recall.
This is the way old-ascii displays when two columns are wrapping -- it
seems pretty confused to me. The headers are displaying newline
markers from the ascii style instead of : indicators in the dividing
line. The : and ; markers are only displayed for the first column, not
the second one.
Maybe since the : and ; markers aren't shown for the final column that
means the expanded mode doesn't have to do anything since the column
is only ever the final column.
++-+
| x |x|
|+y |+ y|
|+z |+ z|
++-+
| x | xxx |
| xx ; |
| xxx: xxx |
| ; xxx |
| x : xxx |
| xx ; xx |
| xxx: xxx |
| ; x |
| x : xxx |
| xx : xx |
| xxx: x |
| : |
| x : xxx |
| xx : xx |
| xxx: x |
| : |
| x : xxx |
| xx : xx |
| xx : x |
| x : |
| xx : xxx |
| xx : xx |
| xx : x |
| xxx: |
| xx : |
| : |
| xx : |
| x : |
| xx : |
| xx |
| xx |
| xxx |
++-+
--
Best regards,
Sergey Muraviov
From 69d845f05864a5d07a90ee20e10a5cb09b78d1d3 Mon Sep 17 00:00:00 2001
From: Sergey Muraviov sergey.k.murav...@gmail.com
Date: Fri, 11 Apr 2014 20:55:17 +0400
Subject: [PATCH] Support for old-ascii mode
---
src/bin/psql/print.c | 144 +++
1 file changed, 122 insertions(+), 22 deletions(-)
diff --git a/src/bin/psql/print.c b/src/bin/psql/print.c
index 79fc43e..2c58cb5 100644
--- a/src/bin/psql/print.c
+++ b/src/bin/psql/print.c
@@ -1234,13 +1234,56 @@ print_aligned_vertical(const printTableContent *cont, FILE *fout)
fprintf(fout, %s\n, cont-title);
}
+ if (cont-opt-format == PRINT_WRAPPED)
+ {
+ 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++)
{
printTextRule pos;
- int line_count,
+ int dline,
+ hline,
dcomplete,
- hcomplete;
+ hcomplete,
+ offset,
+ chars_to_output;
if (cancel_pressed)
break;
@@ -1270,48 +1313,105 @@ print_aligned_vertical(const printTableContent *cont, FILE *fout)
pg_wcsformat((const unsigned char *) *ptr, strlen(*ptr), encoding,
dlineptr, dheight);
- line_count = 0;
+ dline = hline = 0;
dcomplete = hcomplete = 0;
+ offset = 0;
+ chars_to_output = dlineptr[dline].width;
while (!dcomplete || !hcomplete)
{
+ /* Left border */
if (opt_border == 2)
-fprintf(fout, %s , dformat-leftvrule);
+fputs(dformat-leftvrule, fout);
+
+ /* Header */
if (!hcomplete)
{
-fprintf(fout, %-s%*s, hlineptr[line_count].ptr,
- hwidth - hlineptr[line_count].width, );
+int swidth = hwidth - hlineptr[hline].width - 1;
+fputs(hline? format-header_nl_left: , fout);
+fprintf(fout, %-s,
Re: [HACKERS] [PATCH] Negative Transition Aggregate Functions (WIP)
Florian Pflug f...@phlo.org writes: On Apr11, 2014, at 17:09 , Tom Lane t...@sss.pgh.pa.us wrote: Basically, this comes down to a design judgment call, and my judgment is differing from yours. In the absence of opinions from others, I'm going to do it my way. Ok. Are you going to do the necessary changes, or shall I? I'm happy to leave it to you, but if you lack the time I can try to find some. Nah, I'm happy to do the work, since it's me insisting on changing it. Tell me about the special case here again? Should we revisit the issue? ... I don't feel too strongly either way on this one - I initially implemented the exception because I noticed that the NULL handling of some aggregates was broken otherwise, and it seemed simpler to fix this in one place than going over all the aggregates separately. OTOH, when I wrote the docs, I noticed how hard it was to describe the behaviour accurately, which made me like it less and less. And Dean wasn't happy with it at all, so that finally settled it. Yeah, if you can't document the design nicely, it's probably not a good idea :-(. Thanks for the summary. It strikes me that your second point 2) It allows strict aggregates whose state type and input type aren't binary coercible to return NULL if all inputs were NULL without any special coding. As it stands today, this doesn't work without some kind of counter in the state, because the final function otherwise won't know if there were non-NULL inputs or not. Aggregates whose state and input types are binary coercible get around that by setting the initial value to NULL while *still* being strict, and relying on the state replacement behaviour of the aggregate machinery. could be addressed by the initfunc idea, but I'm still not sufficiently excited by that one. Given the problem with internal-type transition values, I think this could only win if there are cases where it would let us use a regular SQL type instead of internal for the transition value; and I'm not sure that there are many/any such cases. 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] [PATCH] Negative Transition Aggregate Functions (WIP)
On Apr11, 2014, at 19:04 , Tom Lane t...@sss.pgh.pa.us wrote: It strikes me that your second point 2) It allows strict aggregates whose state type and input type aren't binary coercible to return NULL if all inputs were NULL without any special coding. As it stands today, this doesn't work without some kind of counter in the state, because the final function otherwise won't know if there were non-NULL inputs or not. Aggregates whose state and input types are binary coercible get around that by setting the initial value to NULL while *still* being strict, and relying on the state replacement behaviour of the aggregate machinery. could be addressed by the initfunc idea, but I'm still not sufficiently excited by that one. Given the problem with internal-type transition values, I think this could only win if there are cases where it would let us use a regular SQL type instead of internal for the transition value; and I'm not sure that there are many/any such cases. Yes, the idea had come up at some point during the review discussion. I agree that it's only worthwhile if it works for state type internal - though I think there ought to be a way to allow that. We could for example simply decree that the initfunc's first parameter must be of type internal if it's return type is, and then just pass NULL for that parameter. What I like about the initfunc idea is that it also naturally extends to ordered-set aggregates, I think it'd be very useful for some possible ordered-set aggregates to received their direct arguments beforehand and not afterwards. But that all seems largely orthogonal to the invtrans patch. best regards, Florian Pflug -- 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] Negative Transition Aggregate Functions (WIP)
Dean Rasheed dean.a.rash...@gmail.com writes: On 10 April 2014 19:54, Tom Lane t...@sss.pgh.pa.us wrote: So if we go with that terminology, perhaps these names for the new CREATE AGGREGATE parameters: initfuncapplies to plain aggregation, mutually exclusive with initcond msfunc (or just mfunc?) forward transition for moving-agg mode mifunc inverse transition for moving-agg mode mstype state datatype for moving-agg mode msspace space estimate for mstype mfinalfunc final function for moving-agg mode minitfunc firsttrans for moving-agg mode minitcond mutually exclusive with minitfunc Yeah, those work for me. I think I prefer mfunc to msfunc, but perhaps that's just my natural aversion to the ms prefix :-) Meh. We've got mstype, and I don't think leaving out the s there feels right. Also, perhaps minvfunc rather than mifunc because i by itself could mean initial. Good point. So with initfuncs out of the picture, we have new CREATE AGGREGATE parameter names msfunc forward transition for moving-agg mode minvfuncinverse transition for moving-agg mode mfinalfunc final function for moving-agg mode mstype state datatype for moving-agg mode msspace space estimate for mstype minitcond initial state value for moving-agg mode and new pg_aggregate columns aggmtransfn | regproc | not null aggminvtransfn| regproc | not null aggmfinalfn | regproc | not null aggmtranstype | oid | not null aggmtransspace| integer | not null aggminitval | text | It's a bit unfortunate that the catalog column names aren't quite on the same page as CREATE AGGREGATE, but it doesn't seem like a good idea to try to fix that now. 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] [PATCH] Negative Transition Aggregate Functions (WIP)
Florian Pflug f...@phlo.org writes: Yes, the idea had come up at some point during the review discussion. I agree that it's only worthwhile if it works for state type internal - though I think there ought to be a way to allow that. We could for example simply decree that the initfunc's first parameter must be of type internal if it's return type is, and then just pass NULL for that parameter. I had thought about that, but it doesn't really work since it'd be violating the strictness spec of the function. 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] Problem with displaying wide tables in psql
Sergey Muraviov wrote: I hope that I realized old-ascii logic correctly. I don't know what you changed here, but I don't think we need to preserve old-ascii behavior in the new code, in any way. If you're mimicking something obsolete and the end result of the new feature is not great, perhaps that should be rethought. Can you please provide sample output for the previous version compared to this new version? What changed? -- Á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] Problem with displaying wide tables in psql
There were no support for wrapping and old-ascii line style in expanded
mode at all.
But now they are.
2014-04-11 21:58 GMT+04:00 Alvaro Herrera alvhe...@2ndquadrant.com:
Sergey Muraviov wrote:
I hope that I realized old-ascii logic correctly.
I don't know what you changed here, but I don't think we need to
preserve old-ascii behavior in the new code, in any way. If you're
mimicking something obsolete and the end result of the new feature is
not great, perhaps that should be rethought.
Can you please provide sample output for the previous version compared
to this new version? What changed?
--
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
--
Best regards,
Sergey Muraviov
\pset expanded on
\pset border 2
\pset columns 20
\pset format wrapped
\pset linestyle unicode
postgres=# select array_to_string(array_agg(repeat('x',n)),E'\n') as x
postgres# y, array_to_string(array_agg(repeat('yy',10-n)),E'\n') as x
postgres# y
postgres# z from (select generate_series(1,10)) as t(n);
┌─[ RECORD 1 ]─┐
│ x↵│ x ↵│
│ y │ xx ↵│
│ │ xxx ↵│
│ │ ↵│
│ │ x ↵│
│ │ xx ↵│
│ │ xxx ↵│
│ │ ↵│
│ │ x ↵│
│ │ xx │
│ x↵│ …│
│ y↵│…yy ↵│
│ z │ …│
│ │…↵│
│ │ …│
│ │…yy ↵│
│ │ ↵│
│ │ yy ↵│
│ │ ↵│
│ │ yy ↵│
│ │ ↵│
│ │ yy ↵│
│ │ │
└───┴──┘
postgres=# \pset linestyle ascii
Line style (linestyle) is ascii.
postgres=# select array_to_string(array_agg(repeat('x',n)),E'\n') as x
y, array_to_string(array_agg(repeat('yy',10-n)),E'\n') as x
y
z from (select generate_series(1,10)) as t(n);
+-[ RECORD 1 ]-+
| x+| x +|
| y | xx +|
| | xxx +|
| | +|
| | x +|
| | xx +|
| | xxx +|
| | +|
| | x +|
| | xx |
| x+| .|
| y+|.yy +|
| z | .|
| |.+|
| | .|
| |.yy +|
| | +|
| | yy +|
| | +|
| | yy +|
| | +|
| | yy +|
| | |
+---+--+
postgres=# \pset linestyle old-ascii
Line style (linestyle) is old-ascii.
postgres=# select array_to_string(array_agg(repeat('x',n)),E'\n') as x
y, array_to_string(array_agg(repeat('yy',10-n)),E'\n') as x
y
z from (select generate_series(1,10)) as t(n);
+-[ RECORD 1 ]-+
| x | x|
|+y : xx |
| : xxx |
| : |
| : x|
| : xx |
| : xxx |
| : |
| : x|
| : xx |
| x | |
|+y ; yy |
|+z : |
| ; |
| : |
| ; yy |
| : |
| : yy |
| : |
| : yy |
| : |
| : yy |
| : |
+---+--+
--
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
Yeah, I think I agree. I'm pretty happy to commit it without old-ascii doing anything special. It looks to me like old-ascii just isn't very useful for a single column output (like expanded mode is implicitly). Maybe that needs to be fixed but then it needs to be fixed for non expanded mode as well. I don't think this new output is very useful dive it just displays the old-ascii info for the header cells. The header cells never wrap and often have a lot of empty lines so it just looks like noise. I do think the earlier change today was important. It looks great now with the wrap and newline indicators in ascii and Unicode mode. Fwiw I've switched my .psqlrc to use Unicode and wrapped=auto be default. It makes psql way more usable. It's even better than my old technique of running it in Emacs and scrolling left and right. I'll take a look at it this evening and will add regression tests (and probably more comments too!) and commit. I want to try adding Unicode regression tests but I'm not sure what the state of the art is for Unicode fallback behaviour on the build farm. I think there are some existing tests we can copy iirc. -- greg
[HACKERS] Problem with txid_snapshot_in/out() functionality
Hackers, the Slony team has been getting seldom reports of a problem with the txid_snapshot data type. The symptom is that a txid_snapshot on output lists the same txid multiple times in the xip list part of the external representation. This string is later rejected by txid_snapshot_in() when trying to determine if a particular txid is visible in that snapshot using the txid_visible_in_snapshot() function. I was not yet able to reproduce this problem in a lab environment. It might be related to subtransactions and/or two phase commit (at least one user is using both of them). The reported PostgreSQL version involved in that case was 9.1. At this point I would find it extremely helpful to sanitize the external representation in txid_snapshot_out() while emitting some NOTICE level logging when this actually happens. I am aware that this does amount to a functional change for a back release, but considering that the _out() generated external representation of an existing binary datum won't pass the type's _in() function, I argue that such change is warranted. Especially since this problem could possibly corrupt a dump. Comments? Jan -- Jan Wieck Senior Software Engineer http://slony.info -- 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] Negative Transition Aggregate Functions (WIP)
On Apr11, 2014, at 19:42 , Tom Lane t...@sss.pgh.pa.us wrote: Florian Pflug f...@phlo.org writes: Yes, the idea had come up at some point during the review discussion. I agree that it's only worthwhile if it works for state type internal - though I think there ought to be a way to allow that. We could for example simply decree that the initfunc's first parameter must be of type internal if it's return type is, and then just pass NULL for that parameter. I had thought about that, but it doesn't really work since it'd be violating the strictness spec of the function. Only if we insist on passing SQL NULL, not if we passed an non-NULL value that happens to be (char*)0. Or we could simply require the initfunc to be non-strict in the case of state type internal. best regards, Florian Pflug -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PostgreSQL in Windows console and Ctrl-C
On Fri, Apr 11, 2014 at 4:42 PM, Andrew Dunstan and...@dunslane.net wrote:
On 04/11/2014 01:35 AM, Amit Kapila wrote:
I don't think this is a complete fix, for example what about platform
where
_CreateRestrictedToken() is not supported. For Example, the current
proposed fix will not work for below case:
if (_CreateRestrictedToken == NULL)
{
/*
* NT4 doesn't have CreateRestrictedToken, so just call ordinary
* CreateProcess
*/
Are we really supporting NT4 any more? Even XP is about to be at end of
support from Microsoft.
In Docs, it is mentioned as Windows (Win2000 SP4 and later).
Now what shall we do with this part of code, shall we keep it as it is and
just fix in other part of code or shall we remove this part of code?
Another thing to decide about this fix is that whether it is okay to fix it
for CTRL+C and leave the problem open for CTRL+BREAK?
(The current option used (CREATE_NEW_PROCESS_GROUP) will handle
only CTRL+C).
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Dynamic Shared Memory stuff
On Wed, Apr 9, 2014 at 9:20 PM, Robert Haas robertmh...@gmail.com wrote: On Wed, Apr 9, 2014 at 7:41 AM, Amit Kapila amit.kapil...@gmail.com wrote: I am just not sure whether it is okay to rearrange the code and call GetLastError() only if returned handle is Invalid (NULL) or try to look for more info. Well, I don't know either. You wrote the Windows portion of this code, so you'll have to decide what's best. If the best practice in this area is to only call GetLastError() if the handle isn't valid, then that's probably what we should do, too. But I can't speak to what the best practice is. I have checked that other place in code also check handle to decide if API has failed. Refer function PGSharedMemoryIsInUse(). So I think fix to call GetLastError() after checking handle is okay. Attached patch fixes this issue. After patch, the server shows below log which is exactly what is expected from test_shm_mq LOG: registering background worker test_shm_mq LOG: starting background worker process test_shm_mq LOG: worker process: test_shm_mq (PID 4888) exited with exit code 1 LOG: unregistering background worker test_shm_mq LOG: registering background worker test_shm_mq LOG: starting background worker process test_shm_mq LOG: worker process: test_shm_mq (PID 3128) exited with exit code 1 LOG: unregistering background worker test_shm_mq With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com fix_dsm_invalid_errcode_issue.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
