Re: [HACKERS] RLS Design
Craig, * Craig Ringer (cr...@2ndquadrant.com) wrote: I was jotting notes about this last sleepless night, and was really glad to see the suggestion of enabling RLS on a table being a requirement for OR-style quals suggested in the thread when I woke. Thanks for your thoughts and input! The only sane way to do OR-ing of multiple rules is to require that tables be switched to deny-by-default before RLS quals can be added to then selectively enable access. Right. The next step is DENY rules that override ALLOW rules, and are also ORed, so any DENY rule overrides any ALLOW rule. Like in ACLs. But that can be a later - I just think room for it should be left in any catalog definition. I'm not convinced regarding DENY rules, and I've seen very little of their use in practice.. The expectation is generally a deny-by-default setups with access granted explicity. My concern with the talk of policies, etc, is with making it possible to impliment this for 9.5. I'd really like to see a robust declarative row-security framework with access policies - but I'm not sure sure it's a good idea to try to assemble policies directly out of low level row security predicates. +1000%- we really need to solidify what should go into 9.5 and get that committed, then work out if there is more we can do in this release cycle. I'm fine with a simple approach to begin with, provided we can build on it moving forward without causing upgrade headaches, provided we can get to where we want to go, of course. Tying things into a policy model that isn't tried or tested might create more problems than it solves unless we implement multiple real-world test cases on top of the model to show it works. To this I would say- the original single-policy-per-table approach has been vetted by actual users to be valuable in their environments. It does not solve all cases, certainly, but it's simple and usable as-is and is the minimum which I would like to see in 9.5. Ideally, we can do better than that, but lets not throw out that win because we insist on a complete solution before it goes into core- because then we'll never get there. For how I think we should be pursuing this in the long run, take a look at how TeraData does it, with heirachical and non-heirachical rules - basically bitmaps or thresholds - that get grouped into access policies. It's a very good way to abstract the low level stuff. If we have low level table predicate filters, we can build this sort of thing on top. I keep thinking that a bitmap or similar might make sense here.. Consider a set of policies where we assign them numbers-per-table, a we can then build a bitmap of them, and then store what bitmap is applied to a given query. That then allows us to compare those bitmaps during plan cache checking to make sure that the policies applied last time are the same which we would be applying now, and therefore the existing cached plan is sufficient. It gets a bit more complicated when you allow AND-vs-OR and groups or hierarchies of policies, of course, but I'd like to think we can come up with a sensible way to represent that to allow for a quick check during plan cache lookup. For 9.5, unless the basics turn out to be way easier than they look and it's all done soon in the release process, surely we should be sticking to just getting the basics of row security in place? Leaving room for enhancement, sure, but sticking to the core feature which to my mind is: Agreed.. - A row security on/off flag for a table; Yes; I like this approach in general. - Room in the catalogs for multiple row security rules per table and a type flag for them. The initial type flag, for ALLOW rules, specifies that all ALLOW rules be ORed together. Works for me. I'm open to a per-table toggle which says AND instead of OR, provided we could implement that sanely and simply. - Syntax for creating and dropping row security predicates. If there can be multiple ones per table they'll need names, like we have with triggers, indexes, etc. Agreed. To Robert's question about having policy names at all, rather than just quals, I feel like we'll need them eventually anyway and having them earlier will simplify things. Additionally, it's simpler to reason about and to manage- one can expect a one-to-many relationship between policies and roles, making it simpler to work with the policy name when associating it it to a role rather than having to remember all of the quals involved. - psql support for listing row security predicates on a table if running as superuser or if you've been explicitly GRANTed access to the catalog table listing row security quals. We need psql support to list the RLS policies.. I don't wish to get into the question about what kind of access that requires though. At least initially, I wouldn't try to limit access to the policies or quals in the catalog... Perhaps we need that but I'd like a bit more discussion
Re: [HACKERS] RLS Design
KaiGai, * Kohei KaiGai (kai...@kaigai.gr.jp) wrote: What I'd like to implement is adjustment of query like: SELECT * FROM t1 WHERE (x like '%abc%') AND (quals by built-in RLS) AND (quals by extension-1) AND ... AND (quals by extension-N); I never mind even if qualifiers in the second block are connected with OR'd manner, however, I want RLS infrastructure to accept additional security models provided by extensions. Would having a table-level 'AND'-vs-'OR' modifier for the RLS policies on that table be sufficient for what you're looking for? That seems a simple enough addition which would still allow more complex groups to be developed later on... Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()
On Wed, Jul 9, 2014 at 5:56 AM, Peter Geoghegan p...@heroku.com wrote: On Tue, Jul 8, 2014 at 1:55 PM, Peter Geoghegan p...@heroku.com wrote: I was worried about the common case where a column name is misspelled that would otherwise be ambiguous, which is why that shows a HINT while the single RTE case doesn't To be clear - I mean a HINT with two suggestions rather than just one. If there are 3 or more equally distant suggestions (even if they're all from different RTEs) we also give no HINT in the proposed patch. Showing up to 2 hints is fine as it does not pollute the error output with perhaps unnecessary messages. That's even more protective than for example git that prints all the equidistant candidates. However I can't understand why it does not show up hints even if there are two equidistant candidates from the same RTE. I think it should. -- Michael
Re: [HACKERS] RLS Design
Robert, * Robert Haas (robertmh...@gmail.com) wrote: If you're going to have predicates be table-level and access grants be table-level, then what's the value in having policies? You could just do: ALTER TABLE table_name GRANT ROW ACCESS TO role_name USING quals; Yes, this would be possible (and is nearly identical to the original patch, except that this includes per-role considerations), however, my thinking is that it'd be simpler to work with policy names rather than sets of quals, to use when mapping to roles, and they would potentially be useful later for other things (eg: for setting up which policies should be applied when, or which should be OR' or ANDd with other policies, or having groups of policies, etc). As I see it, the only value in having policies as separate objects is that you can then, by granting access to the policy, give a particular user a bundle of rights rather than having to grant each right individually. But with this design, you've got to create the policy, then add the quals to it for each table, and then you still have to give access individually for every row, table combination, so what value is the policy object itself providing? To clarify this part- the idea is that you would simply declare a policy name to be a set of quals for a particular table, so you declare them and then map a policy to roles for which it should be used. In this arrangement, you don't declare the policy explicitly before setting the quals, those are done at the same time. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()
On Wed, Jul 9, 2014 at 1:49 AM, Peter Geoghegan p...@heroku.com wrote: 4) This is not nice, could it be possible to remove the stuff from varlena.c? +/* Expand each Levenshtein distance variant */ +#include levenshtein.c +#define LEVENSHTEIN_LESS_EQUAL +#include levenshtein.c +#undef LEVENSHTEIN_LESS_EQUAL Part of the same comment: only varstr_leven_less_equal is used to calculate the distance, should we really move varstr_leven to core? This clearly needs to be reworked as not just a copy-paste of the things in fuzzystrmatch. The flag LEVENSHTEIN_LESS_EQUAL should be let within fuzzystrmatch I think. So there'd be one variant within core and one within contrib/fuzzystrmatch? I don't think that's an improvement. No. The main difference between varstr_leven_less_equal and varstr_leven is the use of the extra argument max_d in the former. My argument here is instead of blindly cut-pasting into core the code you are interested in to evaluate the string distances, is to refactor it to have a unique function, and to let the business with LEVENSHTEIN_LESS_EQUAL within contrib/fuzzystrmatch. This will require some reshuffling of the distance function, but by looking at this patch I am getting the feeling that this is necessary, and should even be split into a first patch for fuzzystrmatch that would facilitate its integration into core. Also why is rest_of_char_same within varlena.c? 5) Do we want hints on system columns as well? I think it's obvious that the answer must be no. That's going to frequently result in suggestions of columns that users will complain aren't even there. If you know about the system columns, you can just get it right. They're supposed to be hidden for most purposes. This may sound ridiculous, but I have already found myself mistyping ctid by tid and cid while working on patches and modules that played with page format, and needing a couple of minutes to understand what was going on (bad morning). I would have welcomed such hints in those cases. -- Michael
Re: [HACKERS] RLS Design
2014-07-09 15:07 GMT+09:00 Stephen Frost sfr...@snowman.net: KaiGai, * Kohei KaiGai (kai...@kaigai.gr.jp) wrote: What I'd like to implement is adjustment of query like: SELECT * FROM t1 WHERE (x like '%abc%') AND (quals by built-in RLS) AND (quals by extension-1) AND ... AND (quals by extension-N); I never mind even if qualifiers in the second block are connected with OR'd manner, however, I want RLS infrastructure to accept additional security models provided by extensions. Would having a table-level 'AND'-vs-'OR' modifier for the RLS policies on that table be sufficient for what you're looking for? That seems a simple enough addition which would still allow more complex groups to be developed later on... Probably, things I'm considering is more simple. If a table has multiple built-in RLS policies, its expression node will be represented as a BoolExpr with OR_EXPR and every policies are linked to its args field, isn't it? We assume the built-in RLS model merges multiple policies by OR manner. In case when an extension want to apply additional security model on top of RLS infrastructure, a straightforward way is to add its own rules in addition to the built-in rules. If extension can get control to modify the above expression node and RLS infrastructure works well on the modified expression node, I think it's sufficient to implement multiple security models on the RLS infrastructure. Thanks, -- 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] Doing better at HINTing an appropriate column within errorMissingColumn()
On Tue, Jul 8, 2014 at 11:10 PM, Michael Paquier michael.paqu...@gmail.com wrote: Showing up to 2 hints is fine as it does not pollute the error output with perhaps unnecessary messages. That's even more protective than for example git that prints all the equidistant candidates. However I can't understand why it does not show up hints even if there are two equidistant candidates from the same RTE. I think it should. Everyone is going to have an opinion on something like that. I was showing deference to the general concern about the absolute (as opposed to relative) quality of the HINTs in the event of equidistant matches by having no two suggestions come from within a single RTE, while still covering the case I thought was important by having two suggestions if there were two equidistant matches across RTEs. I think that's marginally better then what you propose, because your case deals with two equidistant though distinct columns, bringing into question the validity of both would-be suggestions. I'll defer to whatever the consensus is. -- 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] Doing better at HINTing an appropriate column within errorMissingColumn()
On Tue, Jul 8, 2014 at 11:25 PM, Michael Paquier michael.paqu...@gmail.com wrote: So there'd be one variant within core and one within contrib/fuzzystrmatch? I don't think that's an improvement. No. The main difference between varstr_leven_less_equal and varstr_leven is the use of the extra argument max_d in the former. My argument here is instead of blindly cut-pasting into core the code you are interested in to evaluate the string distances, is to refactor it to have a unique function, and to let the business with LEVENSHTEIN_LESS_EQUAL within contrib/fuzzystrmatch. This will require some reshuffling of the distance function, but by looking at this patch I am getting the feeling that this is necessary, and should even be split into a first patch for fuzzystrmatch that would facilitate its integration into core. Also why is rest_of_char_same within varlena.c? Just as before, rest_of_char_same() exists for the express purpose of being called by the two variants varstr_leven_less_equal() and varstr_leven(). Why wouldn't I copy it over too along with those two? Where do you propose to put it? Obviously the existing macro hacks (that I haven't changed) that build the two variants are not terribly pretty, but they're not arbitrary either. They reflect the fact that there is no natural way to add callbacks or something like that. If you pretended that the core code didn't have to care about one case or the other, and that contrib was somehow obligated to hook in its own handler for the !LEVENSHTEIN_LESS_EQUAL case that it now only cares about, then you'd end up with an even bigger mess. Besides, with the patch the core code is calling varstr_leven_less_equal(), which is the bigger of the two variants - it's the LEVENSHTEIN_LESS_EQUAL case, not the !LEVENSHTEIN_LESS_EQUAL case that core cares about for the purposes of building HINTs. In short, I don't know what you mean. What would that reshuffling actually look like? 5) Do we want hints on system columns as well? I think it's obvious that the answer must be no. That's going to frequently result in suggestions of columns that users will complain aren't even there. If you know about the system columns, you can just get it right. They're supposed to be hidden for most purposes. This may sound ridiculous, but I have already found myself mistyping ctid by tid and cid while working on patches and modules that played with page format, and needing a couple of minutes to understand what was going on (bad morning). I think that it's clearly not worth it, even if it is true that a minority sometimes make this mistake. Most users don't know that there are system columns. It's not even close to being worth it to bring that into this. -- 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
[HACKERS] Insert query hangs
Hi all, We have a table in a database DB1 with name Test. We imported this database from another machine. When I fire insert statement it is going in the hang state. Then I created another table with same structure and with same data within it as in table Test. Then I fired the insert statement. It is working fine. I am not able find the reason for this. Can you please help me out on this. This scenario easily reproducible. I have a standalone system and postgresql 9.1 installed on it. Regards Tarkeshwar -- 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] change alter user to be a true alias for alter role
Sorry for the late resp,I will try to update the patch. Jov blog: http:amutu.com/blog http://amutu.com/blog 2014-07-02 4:17 GMT+08:00 Abhijit Menon-Sen a...@2ndquadrant.com: At 2014-06-27 16:11:21 +0200, vik.fear...@dalibo.com wrote: After a week of silence from Jov, I decided to do this myself since it didn't seem very hard. Many frustrating hours of trying to understand why I'm getting shift/reduce conflicts by the hundreds later, I've decided to give up for now. Jov, do you expect to be able to work on the patch along the lines Tom suggested and resubmit during this CF? If not, since Vik gave up on it, it seems to me that it would be best to mark it returned with feedback (which I'll do in a couple of days if I don't hear anything to the contrary). -- Abhijit
Re: [HACKERS] Allowing NOT IN to use ANTI joins
Hi, With further testing I noticed that the patch was not allowing ANTI joins in cases like this: explain select * from a where id not in(select x from b natural join c); I too found this with natural joins and was about to report that. But its good that you found that and fixed it as well. I have reviewed this and didn't find any issues as such. So marking it Ready for Committer. Thanks -- Jeevan B Chalke Principal Software Engineer, Product Development EnterpriseDB Corporation The Enterprise PostgreSQL Company
Re: [HACKERS] psql: show only failed queries
Hi
2014-07-09 7:07 GMT+02:00 Fujii Masao masao.fu...@gmail.com:
On Mon, Jun 30, 2014 at 8:33 PM, Pavel Stehule pavel.steh...@gmail.com
wrote:
2014-06-30 13:01 GMT+02:00 Abhijit Menon-Sen a...@2ndquadrant.com:
At 2014-06-30 12:48:30 +0200, pavel.steh...@gmail.com wrote:
+ para
+ Print a failed SQL commands to standard error output. This is
+ equivalent to setting the variable varnameECHO/varname to
+ literalerrors/literal.
No a, just Print failed SQL commands ….
-option-e/option.
+option-e/option. If set to literalerror/literal then
only
+failed queries are displayed.
Should be errors here, not error.
printf(_( -a, --echo-all echo all input from
script\n));
+ printf(_( -b --echo-errorsecho failed commands sent
to
server\n));
printf(_( -e, --echo-queries echo commands sent to
server\n));
Should have a comma after -b to match other options. Also I would remove
sent to server from the description: echo failed commands is fine.
fixed
$ psql -b
bin/psql: invalid option -- 'b'
Try psql --help for more information.
I got this error. ISTM you forgot to add 'b' into the third argument of
getopt_long in startup.c.
fixed
applicationpsql/application merely prints all queries as
they are sent to the server. The switch for this is
-option-e/option.
+option-e/option. If set to literalerrors/literal then only
+failed queries are displayed.
I think that where failed queries are output should be documented here.
Otherwise users might misunderstand they are output to standard output
like ECHO=all and queries do.
It's better to add The switch for this is option-b/option. into the
doc.
fixed
+else if (strcmp(prev2_wd, \\set) == 0)
+{
+if (strcmp(prev_wd, ECHO) == 0)
+{
+static const char *const my_list[] =
+{none, errors, queries, all, NULL};
+
+COMPLETE_WITH_LIST_CS(my_list);
+}
+else if (strcmp(prev_wd, ECHO_HIDDEN) == 0)
+{
+static const char *const my_list[] =
+{noexec, off, on, NULL};
+
+COMPLETE_WITH_LIST_CS(my_list);
+}
+}
I think that adding tab-completions of psql variables is good, but
adding those of only ECHO and ECHO_HIDDEN seems half-baked.
Probably this part should be split into separate patch.
fixed
please, see updated patch in attachment
Thank you
Regards
Pavel
Regards,
--
Fujii Masao
commit a05543802d000b3e66276b5ff370787d6f5ee7e3
Author: Pavel Stehule pavel.steh...@gooddata.com
Date: Wed Jul 9 14:03:42 2014 +0200
version 04
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 255e8ca..bbe5935 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -73,6 +73,18 @@ PostgreSQL documentation
/varlistentry
varlistentry
+ termoption-b//term
+ termoption--echo-errors//term
+ listitem
+ para
+ Print failed SQL commands to standard error output. This is
+ equivalent to setting the variable varnameECHO/varname to
+ literalerrors/literal.
+ /para
+ /listitem
+/varlistentry
+
+varlistentry
termoption-c replaceable class=parametercommand/replaceable//term
termoption--command=replaceable class=parametercommand/replaceable//term
listitem
@@ -2812,7 +2824,9 @@ bar
literalqueries/literal,
applicationpsql/application merely prints all queries as
they are sent to the server. The switch for this is
-option-e/option.
+option-e/option. If set to literalerrors/literal then only
+failed queries are displayed on standard error output. The switch
+for this is option-b/option.
/para
/listitem
/varlistentry
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index c08c813..676e268 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -995,6 +995,9 @@ SendQuery(const char *query)
results = NULL; /* PQclear(NULL) does nothing */
}
+ if (!OK pset.echo == PSQL_ECHO_ERRORS)
+ psql_error(STATEMENT: %s\n, query);
+
/* If we made a temporary savepoint, possibly release/rollback */
if (on_error_rollback_savepoint)
{
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index 3aa3c16..f8f000f 100644
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -87,6 +87,7 @@ usage(void)
printf(_(\nInput and output options:\n));
printf(_( -a, --echo-all echo all input from script\n));
+ printf(_( -b, --echo-errorsecho failed commands\n));
printf(_( -e, --echo-queries echo commands sent to server\n));
printf(_( -E, --echo-hiddendisplay queries that internal commands generate\n));
printf(_( -L, --log-file=FILENAME send session log to
Re: [HACKERS] psql: show only failed queries
On Wed, Jul 9, 2014 at 9:06 PM, Pavel Stehule pavel.steh...@gmail.com wrote:
Hi
2014-07-09 7:07 GMT+02:00 Fujii Masao masao.fu...@gmail.com:
On Mon, Jun 30, 2014 at 8:33 PM, Pavel Stehule pavel.steh...@gmail.com
wrote:
2014-06-30 13:01 GMT+02:00 Abhijit Menon-Sen a...@2ndquadrant.com:
At 2014-06-30 12:48:30 +0200, pavel.steh...@gmail.com wrote:
+ para
+ Print a failed SQL commands to standard error output. This is
+ equivalent to setting the variable varnameECHO/varname to
+ literalerrors/literal.
No a, just Print failed SQL commands ….
-option-e/option.
+option-e/option. If set to literalerror/literal then
only
+failed queries are displayed.
Should be errors here, not error.
printf(_( -a, --echo-all echo all input from
script\n));
+ printf(_( -b --echo-errorsecho failed commands sent
to
server\n));
printf(_( -e, --echo-queries echo commands sent to
server\n));
Should have a comma after -b to match other options. Also I would
remove
sent to server from the description: echo failed commands is fine.
fixed
$ psql -b
bin/psql: invalid option -- 'b'
Try psql --help for more information.
I got this error. ISTM you forgot to add 'b' into the third argument of
getopt_long in startup.c.
fixed
applicationpsql/application merely prints all queries as
they are sent to the server. The switch for this is
-option-e/option.
+option-e/option. If set to literalerrors/literal then
only
+failed queries are displayed.
I think that where failed queries are output should be documented here.
Otherwise users might misunderstand they are output to standard output
like ECHO=all and queries do.
It's better to add The switch for this is option-b/option. into the
doc.
fixed
+else if (strcmp(prev2_wd, \\set) == 0)
+{
+if (strcmp(prev_wd, ECHO) == 0)
+{
+static const char *const my_list[] =
+{none, errors, queries, all, NULL};
+
+COMPLETE_WITH_LIST_CS(my_list);
+}
+else if (strcmp(prev_wd, ECHO_HIDDEN) == 0)
+{
+static const char *const my_list[] =
+{noexec, off, on, NULL};
+
+COMPLETE_WITH_LIST_CS(my_list);
+}
+}
I think that adding tab-completions of psql variables is good, but
adding those of only ECHO and ECHO_HIDDEN seems half-baked.
Probably this part should be split into separate patch.
fixed
please, see updated patch in attachment
Thanks for updating the patch!
Barring any objection, I will commit this patch except tab-completion part.
I'm not against adding tab-completion support for psql variables, but I'm
not sure if it's good idea or not to treat only one or two variables special
and add tab-completions for them. There are other variables which accept
special argument (e.g., COMP_KEYWORD_CASE) and it's basically worth
adding tab-completion support for 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] psql: show only failed queries
2014-07-09 14:39 GMT+02:00 Fujii Masao masao.fu...@gmail.com:
On Wed, Jul 9, 2014 at 9:06 PM, Pavel Stehule pavel.steh...@gmail.com
wrote:
Hi
2014-07-09 7:07 GMT+02:00 Fujii Masao masao.fu...@gmail.com:
On Mon, Jun 30, 2014 at 8:33 PM, Pavel Stehule pavel.steh...@gmail.com
wrote:
2014-06-30 13:01 GMT+02:00 Abhijit Menon-Sen a...@2ndquadrant.com:
At 2014-06-30 12:48:30 +0200, pavel.steh...@gmail.com wrote:
+ para
+ Print a failed SQL commands to standard error output. This
is
+ equivalent to setting the variable varnameECHO/varname
to
+ literalerrors/literal.
No a, just Print failed SQL commands ….
-option-e/option.
+option-e/option. If set to literalerror/literal
then
only
+failed queries are displayed.
Should be errors here, not error.
printf(_( -a, --echo-all echo all input from
script\n));
+ printf(_( -b --echo-errorsecho failed commands
sent
to
server\n));
printf(_( -e, --echo-queries echo commands sent to
server\n));
Should have a comma after -b to match other options. Also I would
remove
sent to server from the description: echo failed commands is
fine.
fixed
$ psql -b
bin/psql: invalid option -- 'b'
Try psql --help for more information.
I got this error. ISTM you forgot to add 'b' into the third argument of
getopt_long in startup.c.
fixed
applicationpsql/application merely prints all queries as
they are sent to the server. The switch for this is
-option-e/option.
+option-e/option. If set to literalerrors/literal then
only
+failed queries are displayed.
I think that where failed queries are output should be documented here.
Otherwise users might misunderstand they are output to standard output
like ECHO=all and queries do.
It's better to add The switch for this is option-b/option. into
the
doc.
fixed
+else if (strcmp(prev2_wd, \\set) == 0)
+{
+if (strcmp(prev_wd, ECHO) == 0)
+{
+static const char *const my_list[] =
+{none, errors, queries, all, NULL};
+
+COMPLETE_WITH_LIST_CS(my_list);
+}
+else if (strcmp(prev_wd, ECHO_HIDDEN) == 0)
+{
+static const char *const my_list[] =
+{noexec, off, on, NULL};
+
+COMPLETE_WITH_LIST_CS(my_list);
+}
+}
I think that adding tab-completions of psql variables is good, but
adding those of only ECHO and ECHO_HIDDEN seems half-baked.
Probably this part should be split into separate patch.
fixed
please, see updated patch in attachment
Thanks for updating the patch!
Barring any objection, I will commit this patch except tab-completion part.
Thank you
I'm not against adding tab-completion support for psql variables, but I'm
not sure if it's good idea or not to treat only one or two variables
special
and add tab-completions for them. There are other variables which accept
special argument (e.g., COMP_KEYWORD_CASE) and it's basically worth
adding tab-completion support for them.
It can be a second discussion, but I am thinking so anywhere when we can
use autocomplete, then we should it - Although it should not to substitute
documentation, it is fast help about available options, mainly in situation
where variables can hold a relative different content. I can prepare, if
there will not any objection addition patch with complete autocomplete for
all psql variables described in doc.
Regards
Pavel
Regards,
--
Fujii Masao
Re: [HACKERS] [bug fix or improvement?] Correctly place DLLs for ECPG apps in bin folder
From: Alvaro Herrera alvhe...@2ndquadrant.com
I think the suggestion by Peter Eisentraut upthread was pretty
reasonable -- the Makefiles are already aware that they are building a
shared library, so scrape the info off them. The less hardcoded stuff
in the msvc framework, the better.
I overlooked his mail. Do you mean something like this? (I don't know yet
how to express this in Perl)
for each Makefile in under top_dir_in_source_tree or src/interfaces
{
if (Makefile contains SO_MAJOR_VERSION)
{
extract NAME value from Makefile
move lib/lib${NAME}.dll to bin/
}
}
But the source tree contains as many as 206 Makefiles. I'm worried that it
will waste the install time. Should all these Makefiles be examined, or 16
Makefiles in src/interfaces/?
Regards
MauMau
--
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] [BUGS] BUG #9652: inet types don't support min/max
On Mon, Jul 7, 2014 at 6:59 PM, Asif Naeem anaeem...@gmail.com wrote:
Hi Haribabu,
Thank you for sharing the patch. I have spent some time to review the
changes. Overall patch looks good to me, make check and manual testing seems
run fine with it. There seems no related doc/sgml changes ?. Patch added
network_smaller() and network_greater() functions but in PG source code,
general practice seems to be to use “smaller and “larger” as related
function name postfix e.g. timestamp_smaller()/timestamp_larger(),
interval_smaller/interval_larger(), cashsmaller()/cashlarger() etc. Thanks.
Thanks for reviewing the patch.
I corrected the function names as smaller and larger.
and also added documentation changes.
Updated patch attached in the mail.
Regards,
Hari Babu
Fujitsu Australia
*** a/doc/src/sgml/func.sgml
--- b/doc/src/sgml/func.sgml
***
*** 8647,8652 CREATE TYPE rainbow AS ENUM ('red', 'orange', 'yellow', 'green', 'blue', 'purple
--- 8647,8676
row
entry
indexterm
+ primarymax/primary
+ /indexterm
+ literalfunctionmax(typeinet/type, typeinet/type)/function/literal
+ /entry
+ entrytypeinet/type/entry
+ entrylarger of two inet types/entry
+ entryliteralmax('192.168.1.5', '192.168.1.4')/literal/entry
+ entryliteral192.168.1.5/literal/entry
+/row
+row
+ entry
+ indexterm
+ primarymin/primary
+ /indexterm
+ literalfunctionmin(typeinet/type, typeinet/type)/function/literal
+ /entry
+ entrytypeinet/type/entry
+ entrysmaller of two inet types/entry
+ entryliteralmin('192.168.1.5', '192.168.1.4')/literal/entry
+ entryliteral192.168.1.4/literal/entry
+/row
+row
+ entry
+ indexterm
primarynetmask/primary
/indexterm
literalfunctionnetmask(typeinet/type)/function/literal
*** a/src/backend/utils/adt/network.c
--- b/src/backend/utils/adt/network.c
***
*** 471,476 network_ne(PG_FUNCTION_ARGS)
--- 471,499
PG_RETURN_BOOL(network_cmp_internal(a1, a2) != 0);
}
+ Datum
+ network_smaller(PG_FUNCTION_ARGS)
+ {
+ inet *a1 = PG_GETARG_INET_PP(0);
+ inet *a2 = PG_GETARG_INET_PP(1);
+
+ if (network_cmp_internal(a1, a2) 0)
+ PG_RETURN_INET_P(a1);
+ else
+ PG_RETURN_INET_P(a2);
+ }
+
+ Datum
+ network_larger(PG_FUNCTION_ARGS)
+ {
+ inet *a1 = PG_GETARG_INET_PP(0);
+ inet *a2 = PG_GETARG_INET_PP(1);
+
+ if (network_cmp_internal(a1, a2) 0)
+ PG_RETURN_INET_P(a1);
+ else
+ PG_RETURN_INET_P(a2);
+ }
/*
* Support function for hash indexes on inet/cidr.
*/
*** a/src/include/catalog/pg_aggregate.h
--- b/src/include/catalog/pg_aggregate.h
***
*** 164,169 DATA(insert ( 2050 n 0 array_larger ----f f 1073 2277 0 0 0 _nu
--- 164,170
DATA(insert ( 2244 n 0 bpchar_larger ----f f 1060 1042 0 0 0 _null_ _null_ ));
DATA(insert ( 2797 n 0 tidlarger ----f f 2800 27 0 0 0 _null_ _null_ ));
DATA(insert ( 3526 n 0 enum_larger ----f f 3519 3500 0 0 0 _null_ _null_ ));
+ DATA(insert ( 3255 n 0 network_larger ----f f 1205 869 0 0 0 _null_ _null_ ));
/* min */
DATA(insert ( 2131 n 0 int8smaller ----f f 412 20 0 0 0 _null_ _null_ ));
***
*** 186,191 DATA(insert ( 2051 n 0 array_smaller ----f f 1072 2277 0 0 0 _n
--- 187,193
DATA(insert ( 2245 n 0 bpchar_smaller ----f f 1058 1042 0 0 0 _null_ _null_ ));
DATA(insert ( 2798 n 0 tidsmaller ----f f 2799 27 0 0 0 _null_ _null_ ));
DATA(insert ( 3527 n 0 enum_smaller ----f f 3518 3500 0 0 0 _null_ _null_ ));
+ DATA(insert ( 3256 n 0 network_smaller ----f f 1203 869 0 0 0 _null_ _null_ ));
/* count */
DATA(insert ( 2147 n 0 int8inc_any -int8inc_any int8dec_any -f f 0 20 0 20 0 0 0 ));
*** a/src/include/catalog/pg_proc.h
--- b/src/include/catalog/pg_proc.h
***
*** 2120,2125 DATA(insert OID = 922 ( network_le PGNSP PGUID 12 1 0 0 0 f f f t t f i 2 0 1
--- 2120,2129
DATA(insert OID = 923 ( network_gt PGNSP PGUID 12 1 0 0 0 f f f t t f i 2 0 16 869 869 _null_ _null_ _null_ _null_ network_gt _null_ _null_ _null_ ));
DATA(insert OID = 924 ( network_ge PGNSP PGUID 12 1 0 0 0 f f f t t f i 2 0 16 869 869 _null_ _null_ _null_ _null_ network_ge _null_ _null_ _null_ ));
DATA(insert OID = 925 ( network_ne PGNSP PGUID 12 1 0 0 0 f f f t t f i 2 0 16 869 869 _null_ _null_ _null_ _null_ network_ne _null_ _null_ _null_ ));
+ DATA(insert OID = 3257 ( network_larger PGNSP PGUID 12 1 0 0 0 f f f f t f i 2 0 869 869 869 _null_ _null_ _null_ _null_ network_larger _null_ _null_ _null_ ));
+ DESCR(larger of two network types);
+ DATA(insert OID = 3258 (
[HACKERS] FW: [postgres-discuss] Insert query hangs
I have the hang issue in Postgres, When I am going to insert into record in a table. Table structure is enclosed in the discussion. Also we found the issue with unique indexes. When I removed the unique index insert operation is working fine. I need help from this core group that weather this is the bug in the Postgres code or we are using unique index wrongly. Regards Tarkeshwar From: Niklas Andersson Sent: 09 July 2014 18:21 To: M Tarkeshwar Rao; Leo Zhou; postgres-disc...@mailman.lmera.ericsson.se Subject: RE: [postgres-discuss] Insert query hangs I wouldn't advice you to drop the indexes in a production environment, as they are usually very important to get fast queries. Your index doesn't seem to be of much use though, as it looks like you are only indexing one single column that is an integer. It seems as it is not needed. Usually you use indexes with two or more columns to speed up queries, or you join on those columns. If you want to make sure that that column is unique, I would advice you to define it as a primary key. You could also use the keyword unique, but in this case I would prefer to define it as a primary key. Then in order to always get a new, unique integer as a primary key, I would suggest you have a look at CREATE SEQUENCE. The syntax comes from how Oracle does it and it works very nice [1] But, this doesn't explain why your current index is causing problems, becuase it _shouldnt_ ;-/ I think you would need some tools to have a check on the server load. Or have a look at how EXPLAIN works, Unfortunately I don't have that deep competence :-( [1] http://www.postgresql.org/docs/8.1/static/sql-createsequence.html Regards, Niklas From: M Tarkeshwar Rao Sent: Wednesday, 09 July 2014 2:29 PM To: Niklas Andersson; Leo Zhou; postgres-disc...@mailman.lmera.ericsson.semailto:postgres-disc...@mailman.lmera.ericsson.se Subject: RE: [postgres-discuss] Insert query hangs What should I do resolve this issue? Change the structure of Table or I should not create the index. From: Niklas Andersson Sent: 09 July 2014 17:58 To: M Tarkeshwar Rao; Leo Zhou; postgres-disc...@mailman.lmera.ericsson.semailto:postgres-disc...@mailman.lmera.ericsson.se Subject: RE: [postgres-discuss] Insert query hangs Yes, and the more data, the longer it takes to rebuild the index. This is why you drop the indexes during certain copy operations, if you have indexes enabled the copy would take forever. Regards, Niklas From: M Tarkeshwar Rao Sent: Wednesday, 09 July 2014 2:22 PM To: Niklas Andersson; Leo Zhou; postgres-disc...@mailman.lmera.ericsson.semailto:postgres-disc...@mailman.lmera.ericsson.se Subject: RE: [postgres-discuss] Insert query hangs Fine now I understand why it is taking time. Is it possible that insert operation will take time when unique index is already created on the table and table has some data within it? From: Niklas Andersson Sent: 09 July 2014 17:20 To: M Tarkeshwar Rao; Leo Zhou; postgres-disc...@mailman.lmera.ericsson.semailto:postgres-disc...@mailman.lmera.ericsson.se Subject: RE: [postgres-discuss] Insert query hangs Can this be of help [1]? [1] http://www.postgresql.org/docs/9.2/static/sql-createindex.html#SQL-CREATEINDEX-CONCURRENTLY Regards, Niklas From: M Tarkeshwar Rao Sent: Wednesday, 09 July 2014 1:41 PM To: Niklas Andersson; Leo Zhou; postgres-disc...@mailman.lmera.ericsson.semailto:postgres-disc...@mailman.lmera.ericsson.se Subject: RE: [postgres-discuss] Insert query hangs CREATE TABLE eventlogentry ( tableindex integer, object character varying(80), method character varying(80), bgwuser character varying(80), time character(23), realuser character varying(80), host character varying(80), application character varying(80) ) WITH ( OIDS=FALSE ) TABLESPACE mmdata; ALTER TABLE eventlogentry OWNER TO mmsuper; GRANT ALL ON TABLE eventlogentry TO mmsuper; GRANT SELECT ON TABLE eventlogentry TO report; CREATE UNIQUE INDEX ind1_eventlogentry ON eventlogentry USING btree (tableindex ) TABLESPACE mmindex; I am sharing the table structure. When we removed the unique index it is working fine. And when created normal index(not unique) it is working fine. After removing unique index we tried to recreate it but it is giving following infinite logs : concurrent insert in progress within table eventlogentry caveat when building a unique index concurrently is that the uniqueness constraint is already being enforced against other transactions when the second table scan begins Regards Tarkeshwar From: Niklas Andersson Sent: 09 July 2014 16:10 To: M Tarkeshwar Rao; Leo Zhou; postgres-disc...@mailman.lmera.ericsson.semailto:postgres-disc...@mailman.lmera.ericsson.se Subject: RE: [postgres-discuss] Insert query hangs Hi, You have some info on checking on corrupt tables here [1],
Re: [HACKERS] postgresql.auto.conf and reload
On Wed, Jul 9, 2014 at 10:14 AM, Tom Lane t...@sss.pgh.pa.us wrote: Amit Kapila amit.kapil...@gmail.com writes: On Wed, Jul 9, 2014 at 6:40 AM, Mark Kirkwood mark.kirkw...@catalyst.net.nz wrote: Yes, but even well behaved users will see this type of error, because initdb uncomments certain values (ones that are dead certs for being changed via ALTER SYSTEM subsequently like shared_buffers), and then - bang! your next reload gets that your postgresql.conf contains errors message. That is the reason, why I have suggested up-thread that uncommented values should go to postgresql.auto.conf, that will avoid any such observations for a well-behaved user. Uh, what? That sounds like you are proposing that postgresql.conf itself is a dead letter. Which is not going to fly. We had that conversation already. It might sound like that but honestly my intention was to just ease the use for users who just want to rely on Alter System. The right way to fix this is just to avoid processing entries that get overridden later in the configuration file scan. That won't cause anyone to get upset about how their old habits no longer work. Okay. As mentioned upthread, I have fixed by ensuring that for duplicate config params, retain only which comes later during parsing. I think it might have been bit simpler to fix, if we try to fix after parsing is complete, but I think for that we might need to replicate the logic at multiple places. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com avoid_processing_dup_config_params_v1.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] tweaking NTUP_PER_BUCKET
On Tue, Jul 8, 2014 at 5:16 PM, Tomas Vondra t...@fuzzy.cz wrote: Thinking about this a bit more, do we really need to build the hash table on the first pass? Why not to do this: (1) batching - read the tuples, stuff them into a simple list - don't build the hash table yet (2) building the hash table - we have all the tuples in a simple list, batching is done - we know exact row count, can size the table properly - build the table We could do this, and in fact we could save quite a bit of memory if we allocated say 1MB chunks and packed the tuples in tightly instead of palloc-ing each one separately. But I worry that rescanning the data to build the hash table would slow things down too much. Also, maybe we could use a regular linear hash table [1], instead of using the current implementation with NTUP_PER_BUCKET=1. (Although, that'd be absolutely awful with duplicates.) Linear probing is pretty awful unless your load factor is 1. You'd probably want NTUP_PER_BUCKET=0.25, or something like that, which would eat up a lot of memory. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Pg_upgrade and toast tables bug discovered
Uh, I guess we could write some code that iterates over all tables and finds the tables that should have TOAST tables, but don't (because binary-upgrade backend mode suppressed their creation), and adds them. However, that would be a lot of code and might be risky to backpatch. The error is so rare I am not sure it is worth it. I tried to create the failure case and it was very difficult, requiring me to create the problem table first, then some dummy stuff to get the offsets right so the oid would collide. Based on the rareness of the bug, I am not sure it is worth it, but if it is, it would be something only for 9.4 (maybe) and 9.5, not something to backpatch. Another idea would be to renumber empty toast tables that conflict during new toast table creation, when in binary upgrade mode. Since the files are always empty in binary upgrade mode, you could just create a new toast table, repoint the old table to use (because it didn't ask for a specific toast table oid because it didn't need one), and then use that one for the table that actually requested the oid. However, if one is a heap (zero size), and one is an index (8k size), it wouldn't work and you would have to recreate the file system file too. That seems a lot more localized than the other approaches. To me, that sounds vastly more complicated and error-prone than forcing the TOAST tables to be added in a second pass as Andres suggested. But I just work here. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()
Peter Geoghegan wrote: On Tue, Jul 8, 2014 at 11:25 PM, Michael Paquier michael.paqu...@gmail.com wrote: 5) Do we want hints on system columns as well? I think it's obvious that the answer must be no. That's going to frequently result in suggestions of columns that users will complain aren't even there. If you know about the system columns, you can just get it right. They're supposed to be hidden for most purposes. This may sound ridiculous, but I have already found myself mistyping ctid by tid and cid while working on patches and modules that played with page format, and needing a couple of minutes to understand what was going on (bad morning). I think that it's clearly not worth it, even if it is true that a minority sometimes make this mistake. Most users don't know that there are system columns. It's not even close to being worth it to bring that into this. I agree with Peter. This is targeted at regular users. -- Á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] postgresql.auto.conf and reload
Re: Tom Lane 2014-07-08 28635.1404844...@sss.pgh.pa.us Sounds like a 9.5 feature, though. No, ALTER SYSTEM is there now and it needs to work right in its first release. I will go fix this if nobody else does. I'd like to throw in again that imho this should include ALTER SYSTEM RESET (ALL) in 9.4. It wouldn't make much sense to delay this to 9.5, when it's simple functionality that can easily be tested and hence shouldn't break anything in 9.4. Christoph -- c...@df7cb.de | http://www.df7cb.de/ -- 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] Improve bytea error messages
On Tue, Jul 8, 2014 at 12:49 AM, Craig Ringer cr...@2ndquadrant.com wrote: On 07/08/2014 07:44 AM, Robert Haas wrote: On Sun, Jul 6, 2014 at 4:17 AM, Craig Ringer cr...@2ndquadrant.com wrote: After someone reported somewhat less than informative errors in bytea decoding (http://stackoverflow.com/q/24588866/398670) I thought I'd put together a quick patch to improve the errors here. The first two changes seem fine from here, but I think the use of parentheses in the third one likely violates our message style guidelines. Good point. Better? Putting it in ERRHINT is more appropriate. Looks OK to me. Committed. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()
On Wed, Jul 9, 2014 at 2:10 AM, Michael Paquier michael.paqu...@gmail.com wrote: On Wed, Jul 9, 2014 at 5:56 AM, Peter Geoghegan p...@heroku.com wrote: On Tue, Jul 8, 2014 at 1:55 PM, Peter Geoghegan p...@heroku.com wrote: I was worried about the common case where a column name is misspelled that would otherwise be ambiguous, which is why that shows a HINT while the single RTE case doesn't To be clear - I mean a HINT with two suggestions rather than just one. If there are 3 or more equally distant suggestions (even if they're all from different RTEs) we also give no HINT in the proposed patch. Showing up to 2 hints is fine as it does not pollute the error output with perhaps unnecessary messages. That's even more protective than for example git that prints all the equidistant candidates. However I can't understand why it does not show up hints even if there are two equidistant candidates from the same RTE. I think it should. Me, too. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()
On Wed, Jul 9, 2014 at 7:29 AM, Peter Geoghegan p...@heroku.com wrote: Everyone is going to have an opinion on something like that. I was showing deference to the general concern about the absolute (as opposed to relative) quality of the HINTs in the event of equidistant matches by having no two suggestions come from within a single RTE, while still covering the case I thought was important by having two suggestions if there were two equidistant matches across RTEs. I think that's marginally better then what you propose, because your case deals with two equidistant though distinct columns, bringing into question the validity of both would-be suggestions. I'll defer to whatever the consensus is. I agree this is bike shedding. But as long as we're bike shedding... A simple rule is easier for users to understand as well as to code. I would humbly suggest the following: take all the unqualified column names, downcase them, check which ones match most closely the unmatched column. Show the top 3 matches if they're within some arbitrary distance. If they match exactly except for the case and the unmatched column is all lower case add a comment that quoting is required due to the mixed case. Honestly the current logic and the previous logic both seemed reasonable to me. They're not going to be perfect in every case so anything that comes up some some suggestions is fine. -- greg -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] 9.5: UPDATE/DELETE .. ORDER BY .. LIMIT ..
Hi Amit, On 5/14/14 6:41 AM, Amit Kapila wrote: IIUC, the way new design will work is that for new tuple we will now get tableoid+TID, modified column values as an input (for inheritance tables we will get this for all child tables as well) for ModifyTable and get old tuple (which in current case will be provided by MergeAppend or in general by some scan node) from some node beneath the ModifyTable. It then matches the tableoid from old tuple with appropriate tableoid incase of child tables and then form the new tuple for that tableoid using old tuple and modified column values. Having now read the discussion upthread a bit more carefully, I think one of us is confused. AIUI, what was suggested was that the plan nodes below the ModifyTable node would only give you back the modified columns, the tableoid and the TID of the tuple, and no old values at all. This might be a reasonable approach, but I haven't given it that much thought yet. In this case can we safely assume that we will always get tableoid from old tuple, ideally it should be there but just not sure It has to be there or otherwise the scheme won't work. Is there a specific case you're worried about? and another minor point is won't we get TID from old tuple (tuple we get from node beneath ModifyTable), what's the need to pass for new tuple? I don't understand this part, could you rephrase? .marko -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] 9.5: UPDATE/DELETE .. ORDER BY .. LIMIT ..
On 5/11/14 6:47 PM, Tom Lane wrote: The $64 question is whether we'd accept an implementation that fails if the target table has children (ie, is partitioned). That seems to me to not be up to the project's usual quality expectations, but maybe if there's enough demand for a partial solution we should do so. I think that partial support is better than no support unless there are concerns about forwards compatibility. I don't see such concerns having been expressed for this feature. .marko -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] better atomics - v0.5
On Tue, Jul 8, 2014 at 2:21 AM, Amit Kapila amit.kapil...@gmail.com wrote: On Thu, Jul 3, 2014 at 10:56 AM, Amit Kapila amit.kapil...@gmail.com wrote: On Tue, Jul 1, 2014 at 4:10 PM, Andres Freund and...@2ndquadrant.com wrote: Further review of patch: 1. /* * pg_atomic_test_and_set_flag - TAS() * * Acquire/read barrier semantics. */ STATIC_IF_INLINE_DECLARE bool pg_atomic_test_set_flag(volatile pg_atomic_flag *ptr); a. I think Acquire and read barrier semantics aren't equal. With acquire semantics, the results of the operation are available before the results of any operation that appears after it in code which means it applies for both load and stores. Definition of read barrier just ensures loads. So will be right to declare like above in comments? Yeah. Barriers can be by the operations they keep from being reordered (read, write, or both) and by the direction in which they prevent reordering (acquire, release, or both). So in theory there are 9 combinations: read barrier (both directions) read barrier (acquire) read barrier (release) write barrier (both directions) write barrier (acquire) write barrier (both directions) full barrier (both directions) full barrier (acquire) full barrier (release) To make things more confusing, a read barrier plus a write barrier need not equal a full barrier. Read barriers prevent reads from being reordered with other reads, and write barriers keep writes from being reordered with other writes, but neither prevents a read from being reordered relative to a write. A full barrier, however, must prevent all reordering: read/read, read/write, write/read, and write/write. Clarity here is essential. barrier.h only proposed to implement the following: read barrier (both directions), write barrier (both directions), full barrier (both directions) The reason I did that is because it seemed to be more or less what Linux was doing, and it's generally suitable for lock-less algorithms. The acquire and release semantics come into play specifically when you're using barriers to implement locking primitives, which is isn't what I was trying to enable. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW
On Tue, Jul 8, 2014 at 3:07 AM, Etsuro Fujita fujita.ets...@lab.ntt.co.jp wrote: Attached is a WIP patch for the following: /* * postgresPlanForeignModify * Plan an insert/update/delete operation on a foreign table * * Note: currently, the plan tree generated for UPDATE/DELETE will always * include a ForeignScan that retrieves ctids (using SELECT FOR UPDATE) * and then the ModifyTable node will have to execute individual remote * UPDATE/DELETE commands. If there are no local conditions or joins * needed, it'd be better to let the scan node do UPDATE/DELETE RETURNING * and then do nothing at ModifyTable. Room for future optimization ... */ In the patch postgresPlanForeignModify has been modified so that if, in addition to the above condition, the followings are satisfied, then the ForeignScan and ModifyTable node will work that way. - There are no local BEFORE/AFTER triggers. - In UPDATE it's safe to evaluate expressions to assign to the target columns on the remote server. Here is a simple performance test. On remote side: postgres=# create table t (id serial primary key, inserted timestamp default clock_timestamp(), data text); CREATE TABLE postgres=# insert into t(data) select random() from generate_series(0, 9); INSERT 0 10 postgres=# vacuum t; VACUUM On local side: postgres=# create foreign table ft (id integer, inserted timestamp, data text) server myserver options (table_name 't'); CREATE FOREIGN TABLE Unpatched: postgres=# explain analyze verbose delete from ft where id 1; QUERY PLAN --- Delete on public.ft (cost=100.00..162.32 rows=910 width=6) (actual time=1275.255..1275.255 rows=0 loops=1) Remote SQL: DELETE FROM public.t WHERE ctid = $1 - Foreign Scan on public.ft (cost=100.00..162.32 rows=910 width=6) (actual time=1.180..52.095 rows= loops=1) Output: ctid Remote SQL: SELECT ctid FROM public.t WHERE ((id 1)) FOR UPDATE Planning time: 0.112 ms Execution time: 1275.733 ms (7 rows) Patched (Note that the DELETE command has been pushed down.): postgres=# explain analyze verbose delete from ft where id 1; QUERY PLAN --- Delete on public.ft (cost=100.00..162.32 rows=910 width=6) (actual time=0.006..0.006 rows=0 loops=1) - Foreign Scan on public.ft (cost=100.00..162.32 rows=910 width=6) (actual time=0.001..0.001 rows=0 loops=1) Output: ctid Remote SQL: DELETE FROM public.t WHERE ((id 1)) Planning time: 0.101 ms Execution time: 8.808 ms (6 rows) I'll add this to the next CF. Comments are welcome. I haven't looked at the code, but +1 for the general idea. The concept seems good to me, and that's a very large performance improvement. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] RLS Design
KaiGai, * Kohei KaiGai (kai...@kaigai.gr.jp) wrote: 2014-07-09 15:07 GMT+09:00 Stephen Frost sfr...@snowman.net: * Kohei KaiGai (kai...@kaigai.gr.jp) wrote: What I'd like to implement is adjustment of query like: SELECT * FROM t1 WHERE (x like '%abc%') AND (quals by built-in RLS) AND (quals by extension-1) AND ... AND (quals by extension-N); I never mind even if qualifiers in the second block are connected with OR'd manner, however, I want RLS infrastructure to accept additional security models provided by extensions. Would having a table-level 'AND'-vs-'OR' modifier for the RLS policies on that table be sufficient for what you're looking for? That seems a simple enough addition which would still allow more complex groups to be developed later on... Probably, things I'm considering is more simple. If a table has multiple built-in RLS policies, its expression node will be represented as a BoolExpr with OR_EXPR and every policies are linked to its args field, isn't it? We assume the built-in RLS model merges multiple policies by OR manner. In case when an extension want to apply additional security model on top of RLS infrastructure, a straightforward way is to add its own rules in addition to the built-in rules. If extension can get control to modify the above expression node and RLS infrastructure works well on the modified expression node, I think it's sufficient to implement multiple security models on the RLS infrastructure. Another way would be to have a single RLS policy which extensions can modify, sure. That was actually along the lines of the originally proposed patch.. That approach would work if we OR'd multiple policies together too, provided the user took care to only have one policy implemented.. Not sure how easy that would be to work with for extension authors though. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Pg_upgrade and toast tables bug discovered
On Wed, Jul 9, 2014 at 10:13:17AM -0400, Robert Haas wrote: Uh, I guess we could write some code that iterates over all tables and finds the tables that should have TOAST tables, but don't (because binary-upgrade backend mode suppressed their creation), and adds them. However, that would be a lot of code and might be risky to backpatch. The error is so rare I am not sure it is worth it. I tried to create the failure case and it was very difficult, requiring me to create the problem table first, then some dummy stuff to get the offsets right so the oid would collide. Based on the rareness of the bug, I am not sure it is worth it, but if it is, it would be something only for 9.4 (maybe) and 9.5, not something to backpatch. Another idea would be to renumber empty toast tables that conflict during new toast table creation, when in binary upgrade mode. Since the files are always empty in binary upgrade mode, you could just create a new toast table, repoint the old table to use (because it didn't ask for a specific toast table oid because it didn't need one), and then use that one for the table that actually requested the oid. However, if one is a heap (zero size), and one is an index (8k size), it wouldn't work and you would have to recreate the file system file too. That seems a lot more localized than the other approaches. To me, that sounds vastly more complicated and error-prone than forcing the TOAST tables to be added in a second pass as Andres suggested. But I just work here. Agreed. I am now thinking we could harness the code that already exists to optionally add a TOAST table as part of ALTER TABLE ADD COLUMN. We would just need an entry point to call it from pg_upgrade, either via an SQL command that checks (and hopefully doesn't do anything else), or a C function that does it, e.g. VACUUM would be trivial to run on every database, but I don't think it tests that; is _could_ in binary_upgrade mode. However, the idea of having a C function plug into the guts of the server and call internal functions makes me uncomforable. The code already called via pg_restore would only need to suppress TOAST table creation if a heap oid is supplied but a TOAST table oid is not. -- 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] postgresql.auto.conf and reload
On 07/08/2014 08:18 PM, Amit Kapila wrote: Yes, but even well behaved users will see this type of error, because initdb uncomments certain values (ones that are dead certs for being changed via ALTER SYSTEM subsequently like shared_buffers), and then - bang! your next reload gets that your postgresql.conf contains errors message. That is the reason, why I have suggested up-thread that uncommented values should go to postgresql.auto.conf, that will avoid any such observations for a well-behaved user Not an effective solution for three reasons: 1) Some users will copy over their older pg.conf's to 9.4, which will already have shared_buffers uncommented; 2) Some distros ship their own pg.conf; 3) Doesn't solve the issue of overlapping files in conf.d, which is the same problem. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] postgresql.auto.conf and reload
On 07/08/2014 06:10 PM, Mark Kirkwood wrote: On 09/07/14 05:13, Josh Berkus wrote: On 07/06/2014 01:27 AM, Christoph Berg wrote: Another could be that during initdb all the uncommented settings be written to postgresql.auto.conf file rather than to postgresql.conf. I think we can do this by changing code in initdb.c-setup_config(). This will ensure that unless user is changing settings in a mixed way (some by Alter System and some manually by editing postgresql.conf), there will no such problem. There is no reasonable way for us to prevent issues for users who are manually changing both pg.conf and pg.auto.conf. Users should stick to one or the other method of management (or, thirdly, using conf.d); if they mix methods, we can't prevent confusion at restart time and we shouldn't try. Yes, but even well behaved users will see this type of error, because initdb uncomments certain values (ones that are dead certs for being changed via ALTER SYSTEM subsequently like shared_buffers), and then - bang! your next reload gets that your postgresql.conf contains errors message. Actually, my response was based on misreading Berg's suggestion; I thought he was suggesting that we would try to disentangle manual changes to both, whereas he was suggesting the opposite. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] IMPORT FOREIGN SCHEMA statement
Michael Paquier michael.paqu...@gmail.com writes: With that, I am marking this patch as ready for committer. I've started looking at this patch. I wonder whether it's really such a great idea to expect the FDW to return a list of parsetrees for CREATE FOREIGN TABLE commands; that seems like a recipe for breakage anytime we change the parsetree representation, say add a field to ColumnDef. The alternative I'm thinking about is to have the FDW pass back a list of strings, which would be textual CREATE FOREIGN TABLE commands. This seems like it'd be more robust and in most cases not any harder for the FDW to generate; moreover, it would greatly improve the quality of error reporting anytime there was anything wrong with what the FDW did. As against that, you could point out that we make FDWs deal with parsetrees when doing planning. But the important difference there is that they're mostly *reading* the parsetrees, not building new ones from scratch, so there's much less opportunity for errors of omission. Comments? 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] add line number as prompt option to psql
On Mon, Jul 7, 2014 at 8:33 PM, Jeevan Chalke jeevan.cha...@enterprisedb.com wrote: Hi, Found two (A and B) issues with latest patch: A: -- Set prompts postgres=# \set PROMPT1 '%/[%l]%R%# ' postgres[1]=# \set PROMPT2 '%/[%l]%R%# ' postgres[1]=# postgres[1]=# select postgres[2]-# * postgres[3]-# from postgres[4]-# abc; ERROR: relation abc does not exist LINE 4: abc; ^ Now I used \e to edit the source. Deleted last line i.e. abc; and returned to prompt, landed at line 4, typed abc; postgres[1]=# \e postgres[4]-# abc; ERROR: relation abc does not exist LINE 5: abc; ^ In above steps, error message says LINE 5, where as on prompt abc is at line 4. postgres[1]=# select * from abc; ERROR: relation abc does not exist LINE 4: abc; ^ Here I again see error at line 4. Something fishy. Please investigate. Looks like bug in LINE numbering in error message, not sure though. But with prompt line number feature, it should be sync to each other. B: However, I see that you have removed the code changes related to INT_MAX. Why? I have set cur_line to INT_MAX - 2 and then observed that after 2 lines I start getting negative numbers. Thank you for reviewing the patch. I have revised the patch, and attached. A: But with prompt line number feature, it should be sync to each other. This patch can handle this case. B: However, I see that you have removed the code changes related to INT_MAX. Why? I had mistake to remove them. I added them to latest patch. Regards, --- Sawada Masahiko psql-line-number_v3.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_resetxlog to clear backup start/end locations.
On Wed, Jun 25, 2014 at 02:08:26PM +0900, Kyotaro HORIGUCHI wrote: Hello, thank you for the suggestion. I dont' touch what '-n' option shows and rewrite documents for the option a bit. And '-n' won't show the changes of backup location. === There are some changes which haven't been shown by '-n' option, even not displayed at all. I think these should be shown by '-n'. I suppose this is a kind of bug but fixing it seems to be a kind of 'feature change'.. Any suggestions? This seems the problem of the document and the help message of -n option. According to the source code, -n option displays only the values that -e, -l, -m -o, -O, and -x options change. The values -f option forcibly changes are not be shown in -n option. I'm not sure if this is an oversight in 108e399... The html(sgml) document says that, === share/doc/html/app-pgresetxlog.html | The -n (no operation) option instructs pg_resetxlog to print | the values reconstructed from pg_control and values about to be | changed, and then exit without modifying anything. This is | mainly a debugging tool, but can be useful as a sanity check | before allowing pg_resetxlog to proceed for real. This seems to have same meaning to the help message. For debugging use also supports your way of understanding the option, I suppose. Anyway, I think that making -n option display all the values that -f option changes would be useful. But since that's not a bugfix, we should apply it only in HEAD. Agreed. Is this a TODO item? -- 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
[HACKERS] how to find the order of joins from Explain command XML plan output in PostgreSQL
EXPLAIN (format XML) command in PostgreSQL9.3.4 gives the plan chosen by the optimizer in XML format. In my program, I have to extract certain data about optimizer plan from this XML output. I am using *LibXML2* library for parsing the XML. I had successfully extracted information about which relations are involved and what joins are used by parsing the XML. But I am *unable to extract the* *order of joining the relations from the XML output*. I conceptually understood that the reverse level order traversal of binary tree representation of the XML plan will give correct ordering of joins applied. But I could not figure out how do I get that from the XML? Does libXML2 support anything of this sort? If not how should I proceed to tackle this?
Re: [HACKERS] tweaking NTUP_PER_BUCKET
On 9.7.2014 16:07, Robert Haas wrote: On Tue, Jul 8, 2014 at 5:16 PM, Tomas Vondra t...@fuzzy.cz wrote: Thinking about this a bit more, do we really need to build the hash table on the first pass? Why not to do this: (1) batching - read the tuples, stuff them into a simple list - don't build the hash table yet (2) building the hash table - we have all the tuples in a simple list, batching is done - we know exact row count, can size the table properly - build the table We could do this, and in fact we could save quite a bit of memory if we allocated say 1MB chunks and packed the tuples in tightly instead of palloc-ing each one separately. But I worry that rescanning the data to build the hash table would slow things down too much. OK. I think we should shoot for no negative impact on well estimated plans, and the rescans might violate that. I need to think about this for some time, but my current plan is this: (1) size the buckets for NTUP_PER_BUCKET=1 (and use whatever number of batches this requires) (2) build the batches as in the current patch (3) if the hash table is too small, resize it Which is not really different from the current patch. Also, maybe we could use a regular linear hash table [1], instead of using the current implementation with NTUP_PER_BUCKET=1. (Although, that'd be absolutely awful with duplicates.) Linear probing is pretty awful unless your load factor is 1. You'd probably want NTUP_PER_BUCKET=0.25, or something like that, which would eat up a lot of memory. My experience is that 0.5-0.75 load factor is quite OK, and NTUP_PER_BUCKET=1 gives you ~0.75 load on average, so it's not that different. Anyway, the inability to handle duplicies reasonably is probably a show-stopper. Tomas -- 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] IMPORT FOREIGN SCHEMA statement
Tom Lane wrote: Michael Paquier michael.paqu...@gmail.com writes: With that, I am marking this patch as ready for committer. I've started looking at this patch. I wonder whether it's really such a great idea to expect the FDW to return a list of parsetrees for CREATE FOREIGN TABLE commands; that seems like a recipe for breakage anytime we change the parsetree representation, say add a field to ColumnDef. The alternative I'm thinking about is to have the FDW pass back a list of strings, which would be textual CREATE FOREIGN TABLE commands. .oO(json blobs as in the DDL deparse patch ...) (I don't know if they are really suitable. I have no idea how this patch works.) -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: how to find the order of joins from Explain command XML plan output in PostgreSQL
csrajmohan wrote EXPLAIN (format XML) command in PostgreSQL9.3.4 gives the plan chosen by the optimizer in XML format. In my program, I have to extract certain data about optimizer plan from this XML output. I am using *LibXML2* library for parsing the XML. I had successfully extracted information about which relations are involved and what joins are used by parsing the XML. But I am *unable to extract the* *order of joining the relations from the XML output*. I conceptually understood that the reverse level order traversal of binary tree representation of the XML plan will give correct ordering of joins applied. But I could not figure out how do I get that from the XML? Does libXML2 support anything of this sort? If not how should I proceed to tackle this? So, since nothing better has been forthcoming in your other two posts on this topic I'll just say that likely you will have much better luck using SAX-based processing as opposed to DOM-based processing. I seriously doubt native/core PostgreSQL facilities will allow you to do what you desire. As you said, hierarchy and physical output order determines the order of joining within the planner so you have to capture and track such relational information during your processing - which is made much easier if you simply traverse the output node-by-node exactly as a SAX based parser does. Though pgAdminIII has a visual query display that you might look at for inspiration. David J. -- View this message in context: http://postgresql.1045698.n5.nabble.com/how-to-find-the-order-of-joins-from-Explain-command-XML-plan-output-in-PostgreSQL-tp5811053p5811056.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.com. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] 9.5: UPDATE/DELETE .. ORDER BY .. LIMIT ..
On Tue, Jun 24, 2014 at 04:08 AM, Heikki Linnakangas hlinnakan...@vmware.com wrote: IMHO this needs to work with inheritance if we are to accept it. It would be a rather strange limitation for no apparent reason, other than that we didn't bother to implement it. It doesn't seem very difficult in theory to add the table OID to the plan as a junk column, and use that in the ModifyTable node to know which table a row came from. I can have a go at that, but I'm somewhat afraid of the performance implications this might have. And it's not just users of this feature that would pay the penalty, it would be everyone. Per the docs in the patch: + para + If the literalLIMIT/ (or literalFETCH FIRST/) clause + is present, processing will stop after the system has attempted + to delete the specified amount of rows. In particular, if a row + was concurrently changed not to match the given literalWHERE/ + clause, it will count towards the literalLIMIT/ despite it + not being actually deleted. Unlike in literalSELECT/, the + literalOFFSET/literal clause is not available in + literalDELETE/. + /para That behavior with READ COMMITTED mode and concurrent changes is surprising. Do we really want it to behave like that, and if so, why? Oh, oops. Looks like I didn't submit the latest version of the patch for the commit fest, where I had fixed the documentation. It doesn't work that way anymore, as we really don't want it to work that way. Why is OFFSET not supported? Not that I care much for that, but I'm curious. I thought it seemed weird. But it's supported for FOR UPDATE, so maybe we should support it here as well. ♜ -- 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] [REVIEW] Re: Compression of full-page-writes
But though the code looks better locally than before, the larger problem
is that this is still unsafe. As Pavan pointed out, XLogInsert is called
from inside critical sections, so we can't allocate memory here.
Could you look into his suggestions of other places to do the
allocation, please?
If I understand correctly , the reason memory allocation is not allowed in
critical section is because OOM error in critical section can lead to PANIC.
This patch does not report an OOM error on memory allocation failure
instead it proceeds without compression of FPW if sufficient memory is not
available for compression. Also, the memory is allocated just once very
early in the session. So , the probability of OOM seems to be low and even
if it occurs it is handled as mentioned above.
Though Andres said we cannot use malloc in critical section, the memory
allocation done in the patch does not involve reporting OOM error in case
of failure. IIUC, this eliminates the probability of PANIC in critical
section. So, I think keeping this allocation in critical section should be
fine. Am I missing something?
Thank you,
Rahila Syed
On Mon, Jul 7, 2014 at 4:43 PM, Rahila Syed rahilasye...@gmail.com wrote:
Thank you for review comments.
There are still numerous formatting changes required, e.g. spaces around
= and correct formatting of comments. And git diff --check still has
a few whitespace problems. I won't point these out one by one, but maybe
you should run pgindent
I will do this.
Could you look into his suggestions of other places to do the
allocation, please?
I will get back to you on this
Wouldn't it be better to set
bkpb-block_compression = compress_backup_block;
once earlier instead of setting it that way once and setting it to
BACKUP_BLOCK_COMPRESSION_OFF in two other places
Yes.
If you're using VARATT_IS_COMPRESSED() to detect compression, don't you
need SET_VARSIZE_COMPRESSED() in CompressBackupBlock? pglz_compress()
does it for you, but the other two algorithms don't.
Yes we need SET_VARSIZE_COMPRESSED. It is present in wrappers around
snappy and LZ4 namely pg_snappy_compress and pg_LZ4_compress.
But now that you've added bkpb.block_compression, you should be able to
avoid VARATT_IS_COMPRESSED() altogether, unless I'm missing something.
What do you think?
You are right. It can be removed.
Thank you,
On Fri, Jul 4, 2014 at 9:35 PM, Abhijit Menon-Sen a...@2ndquadrant.com
wrote:
At 2014-07-04 21:02:33 +0530, a...@2ndquadrant.com wrote:
+/*
+ */
+static const struct config_enum_entry
backup_block_compression_options[] = {
Oh, I forgot to mention that the configuration setting changes are also
pending. I think we had a working consensus to use full_page_compression
as the name of the GUC. As I understand it, that'll accept an algorithm
name as an argument while we're still experimenting, but eventually once
we select an algorithm, it'll become just a boolean (and then we don't
need to put algorithm information into BkpBlock any more either).
-- Abhijit
Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()
On Wed, Jul 9, 2014 at 8:08 AM, Greg Stark st...@mit.edu wrote: A simple rule is easier for users to understand as well as to code. I would humbly suggest the following: take all the unqualified column names, downcase them, check which ones match most closely the unmatched column. Show the top 3 matches if they're within some arbitrary distance. That's harder than it sounds. You need even more translatable strings for variant ereports(). I don't think that an easy to understand rule is necessarily of much value - I'm already charging half price for deletion because I found representative errors more useful in certain cases by doing so. I think we want something that displays the most useful suggestion as often as is practically possible, and does not display unhelpful suggestions to the extent that it's practical to avoid them. Plus, as I mentioned, I'm keen to avoid adding more stuff to scanRTEForColumn() than I already have. Honestly the current logic and the previous logic both seemed reasonable to me. They're not going to be perfect in every case so anything that comes up some some suggestions is fine. I think that the most recent revision is somewhat better due to the feedback of Tom and Robert. I didn't feel as strongly as they did about erring on the side of not showing a HINT, but I think the most recent revision is a good compromise. But yes, at this point we're certainly chasing diminishing returns. There are almost any number of variants of this basic idea that could be suggested. -- 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] Doing better at HINTing an appropriate column within errorMissingColumn()
On Wed, Jul 9, 2014 at 8:06 AM, Robert Haas robertmh...@gmail.com wrote: Showing up to 2 hints is fine as it does not pollute the error output with perhaps unnecessary messages. That's even more protective than for example git that prints all the equidistant candidates. However I can't understand why it does not show up hints even if there are two equidistant candidates from the same RTE. I think it should. Me, too. The idea is that each RTE gets one best suggestion, because if there are two best suggestions within an RTE they're probably both wrong. Whereas across RTEs, it's probably just that there is a foreign key relationship between the two (and the user accidentally failed to qualify the particular column of interest on top of the misspelling, a qualification that would be sufficient to have the code prefer the qualified-but-misspelled column). Clearly if I was to do what you suggest it would be closer to a wild guess, and Tom has expressed concerns about that. Now, I don't actually ensure that the column names of the two columns (each from separate RTEs) are identical save for their would-be alias, but that's just a consequence of the implementation. Also, as I've mentioned, I don't want to put more stuff in scanRTEForColumn() than I already have, due to your earlier concern about adding clutter. I think we're splitting hairs at this point, and frankly I'll do it that way if it gets the patch closer to being committed. While I thought it was important to get the unqualified and misspelled case right (which I did in the first revision, but perhaps at the expense of Tom's concern about absolute suggestion quality), I don't feel strongly about this detail either way. -- 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] Doing better at HINTing an appropriate column within errorMissingColumn()
Peter Geoghegan wrote: On Wed, Jul 9, 2014 at 8:08 AM, Greg Stark st...@mit.edu wrote: A simple rule is easier for users to understand as well as to code. I would humbly suggest the following: take all the unqualified column names, downcase them, check which ones match most closely the unmatched column. Show the top 3 matches if they're within some arbitrary distance. That's harder than it sounds. You need even more translatable strings for variant ereports(). Maybe it is possible to rephrase the message so that the translatable part doesn't need to concern with how many suggestions there are. For instance something like perhaps you meant a name from the following list: foo, bar, baz. Couple with the errmsg_plural stuff, you then don't need to worry too much about providing different strings for 1, 2, N suggestions. -- Á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] Minmax indexes
Heikki Linnakangas wrote: On 06/23/2014 08:07 PM, Alvaro Herrera wrote: I feel that the below would nevertheless be simpler: I wonder if it would be simpler to just always store the revmap pages in the beginning of the index, before any other pages. Finding the revmap page would then be just as easy as with a separate fork. When the table/index is extended so that a new revmap page is needed, move the existing page at that block out of the way. Locking needs some consideration, but I think it would be feasible and simpler than you have now. Moving index items around is not easy, because you'd have to adjust the revmap to rewrite the item pointers. Hmm. Two alternative schemes come to mind: 1. Move each index tuple off the page individually, updating the revmap while you do it, until the page is empty. Updating the revmap for a single index tuple isn't difficult; you have to do it anyway when an index tuple is replaced. (MMTuples don't contain a heap block number ATM, but IMHO they should, see below) 2. Store the new block number of the page that you moved out of the way in the revmap page, and leave the revmap pointers unchanged. The revmap pointers can be updated later, lazily. Both of those seem pretty straightforward. The trouble I have with moving blocks around to make space, is that it would cause the index to have periodic hiccups to make room for the new revmap pages. One nice property that these indexes are supposed to have is that the effect into insertion times should be pretty minimal. That would cease to be the case if we have to do your proposed block moves. ISTM that when the old tuple cannot be updated in-place, the new index tuple is inserted with mm_doinsert(), but the old tuple is never deleted. It's deleted by the next vacuum. Ah I see. Vacuum reads the whole index, and builds an in-memory hash table that contains an ItemPointerData for every tuple in the index. Doesn't that require a lot of memory, for a large index? That might be acceptable - you ought to have plenty of RAM if you're pushing around multi-terabyte tables - but it would nevertheless be nice to not have a hard requirement for something as essential as vacuum. I guess if you're expecting that pages_per_range=1 is a common case, yeah it might become an issue eventually. One idea I just had is to have a bit for each index tuple, which is set whenever the revmap no longer points to it. That way, vacuuming is much easier: just scan the index and delete all tuples having that bit set. No need for this hash table stuff. I am still concerned with adding more overhead whenever a page range is modified, so that insertions in the table continue to be fast. If we're going to dirty the index every time, it might not be so fast anymore. But then maybe I'm worrying about nothing; I will have to measure how slower it is. Wouldn't it be simpler to remove the old tuple atomically with inserting the new tuple and updating the revmap? Or at least mark the old tuple as deletable, so that vacuum can just delete it, without building the large hash table to determine that it's deletable. Yes, it might be simpler, but it'd require dirtying more pages on insertions (and holding more page-level locks, for longer. Not good for concurrent access). I'm quite surprised by the use of LockTuple on the index tuples. I think the main reason for needing that is the fact that MMTuple doesn't store the heap (range) block number that the tuple points to: LockTuple is required to ensure that the tuple doesn't go away while a scan is following a pointer from the revmap to it. If the MMTuple contained the BlockNumber, a scan could check that and go back to the revmap if it doesn't match. Alternatively, you could keep the revmap page locked when you follow a pointer to the regular index page. There's the intention that these accesses be kept as concurrent as possible; this is why we don't want to block the whole page. Locking individual TIDs is fine in this case (which is not in SELECT FOR UPDATE) because we can only lock a single tuple in any one index scan, so there's no unbounded growth of the lock table. I prefer not to have BlockNumbers in index tuples, because that would make them larger for not much gain. That data would mostly be redundant, and would be necessary only for vacuuming. -- Á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] Doing better at HINTing an appropriate column within errorMissingColumn()
On Wed, Jul 9, 2014 at 2:19 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: That's harder than it sounds. You need even more translatable strings for variant ereports(). Maybe it is possible to rephrase the message so that the translatable part doesn't need to concern with how many suggestions there are. For instance something like perhaps you meant a name from the following list: foo, bar, baz. Couple with the errmsg_plural stuff, you then don't need to worry too much about providing different strings for 1, 2, N suggestions. That's not really the problem. I already have a lot of things to test in each of the two ereport() calls. More importantly, showing the closet, say, 3 matches under an arbitrary distance does not weigh concerns about that indicating that they're all bad. It's not like bash tab completion - if there is one best match, that's probably because that's what the user meant. Whereas if there are two or more within a single RTE, that's probably because both are unhelpful. They both happened to require the same number of substitutions to get to, while not being quite bad enough matches to be excluded by the final check against a normalized distance threshold (the final check that prevents ludicrous suggestions). The fact that there were multiple equally plausible candidates (that are not identically named and just from different RTEs) tells us plenty, unlike with tab completion. It's not hard for one column to be a better match than another, and so it doesn't seem unreasonable to insist upon that within a single RTE where they cannot be identical, since a conservative approach seems to be what is generally favored. In any case I'm just trying to weigh everyone's concerns here. I hope it's actually possible to compromise, but right now I don't know what I can do to make useful progress. -- 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] Doing better at HINTing an appropriate column within errorMissingColumn()
On Tue, Jul 8, 2014 at 6:58 AM, Michael Paquier michael.paqu...@gmail.com wrote: 6) Sometimes no hints are returned... Even in simple cases like this one: =# create table foo (aa int, bb int); CREATE TABLE =# select ab from foo; ERROR: 42703: column ab does not exist LINE 1: select ab from foo; ^ LOCATION: errorMissingColumn, parse_relation.c:3123 In this example, it seems obvious that both aa and bb should be suggested when they are not. But what if there were far more columns, as might be expected in realistic cases (suppose all other columns have at least 3 characters)? That's another kettle of fish. The assumption that it's probably one of those two equally distant columns is now on very shaky ground. After all, the user can only have meant one particular column. If we apply a limited kind of Turing test to this second case, how does the most recent revision's algorithm do? What would a human suggest? I'm pretty sure the answer is that the human would shrug. Maybe he or she would say I guess you might have meant one of either aa or bb, but that really isn't obvious at all. That doesn't inspire much confidence. Now, maybe I should be more optimistic about it being one of the two because there are only two possibilities to begin with. That seems pretty dubious, though. In general I find it much more plausible based on what we know that the user should rethink everything. And, as Tom pointed out, showing nothing conveys something in itself once users have been trained to expect something. -- 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] Minmax indexes
On 07/09/2014 02:16 PM, Alvaro Herrera wrote: The way it works now, each opclass needs to have three support procedures; I've called them getOpers, maybeUpdateValues, and compare. (I realize these names are pretty bad, and will be changing them.) I kind of like maybeUpdateValues. Very ... NoSQL-ish. Maybe update the values, maybe not. ;-) -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Minmax indexes
On Wed, Jul 9, 2014 at 2:16 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: All this being said, I'm sticking to the name Minmax indexes. There was a poll in pgsql-advocacy http://www.postgresql.org/message-id/53a0b4f8.8080...@agliodbs.com about a new name, but there were no suggestions supported by more than one person. If a brilliant new name comes up, I'm open to changing it. How about summarizing indexes? That seems reasonably descriptive. -- 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] IMPORT FOREIGN SCHEMA statement
On Thu, Jul 10, 2014 at 2:30 AM, Tom Lane t...@sss.pgh.pa.us wrote: Michael Paquier michael.paqu...@gmail.com writes: With that, I am marking this patch as ready for committer. I've started looking at this patch. I wonder whether it's really such a great idea to expect the FDW to return a list of parsetrees for CREATE FOREIGN TABLE commands; that seems like a recipe for breakage anytime we change the parsetree representation, say add a field to ColumnDef. The alternative I'm thinking about is to have the FDW pass back a list of strings, which would be textual CREATE FOREIGN TABLE commands. This seems like it'd be more robust and in most cases not any harder for the FDW to generate; moreover, it would greatly improve the quality of error reporting anytime there was anything wrong with what the FDW did. Agreed. Modifying postgres_fdw portion to do so would not take long. As against that, you could point out that we make FDWs deal with parsetrees when doing planning. But the important difference there is that they're mostly *reading* the parsetrees, not building new ones from scratch, so there's much less opportunity for errors of omission. Comments? The SQL-MED spec talks only about foreign tables when importing, It would be good to keep the checks on CreateTableForeignStmt after parsing the strings, which is what I imagine server would do after taking back the list of strings from FDW before creating the objects. Regards, -- Michael
Re: [HACKERS] Minmax indexes
Josh Berkus wrote: On 07/09/2014 02:16 PM, Alvaro Herrera wrote: The way it works now, each opclass needs to have three support procedures; I've called them getOpers, maybeUpdateValues, and compare. (I realize these names are pretty bad, and will be changing them.) I kind of like maybeUpdateValues. Very ... NoSQL-ish. Maybe update the values, maybe not. ;-) :-) Well, that's exactly what happens. If we insert a new tuple into the table, and the existing summarizing tuple (to use Peter's term) already covers it, then we don't need to update the index tuple at all. What this name doesn't say is what values are to be maybe-updated. -- Á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] Minmax indexes
On Wed, Jul 9, 2014 at 10:16 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: there is no hardcoded assumption on the number of index values stored per heap column, so it is possible to build an opclass that stores a bounding box column for a geometry heap column, for instance. I think the more Postgresy thing to do is to store one datum per heap column. It's up to the opclass to find or make a composite data type that stores all the necessary state. So you could make a minmax_accum data type like NumericAggState in numeric.c:numeric_accum() or the array of floats in float8_accum. For a bounding box a 2d geometric min/max index could use the box data type for example. The way you've done it seems more convenient but there's something to be said for using the same style for different areas. A single bounding box accumulator function would probably suffice for both an aggregate and index opclass for example. But this sounds pretty great. I think it would let me do the bloom filter index I had in mind fairly straightforwardly. The result would be something very similar to a bitmap index. I'm not sure if there's a generic term that includes bitmap indexes or other summary functions like bounding boxes (which min/max is basically -- a 1D bounding box). Thanks a lot for listening and being so open, I think what you describe is a lot more flexible than what you had before and I can see some pretty great things coming out of it (including min/max itself of course). -- greg -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] IMPORT FOREIGN SCHEMA statement
Another thing ... The SQL-MED standard says that IMPORT FOREIGN SCHEMA ... LIMIT TO (a,b,c) should throw an error if not all of a,b,c exist as tables in the remote server. It's rather sloppily worded, though, such that it's not clear whether an error is also expected for EXCEPT (a,b,c) when not all of those names exist. It seems to me that the whole thing is badly thought out and it would be better not to complain for nonexistent names in either type of list. The use of LIMIT seems to me to imply that the list is a filter, not an exact set of names that must be present. Comments? 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] things I learned from working on memory allocation
On Mon, Jul 7, 2014 at 7:29 PM, Peter Geoghegan p...@heroku.com wrote: I do think that's a problem with our sort implementation, but it's not clear to me whether it's *more* of a problem for parallel sort than it is for single-backend sort. If you'll forgive me for going on about my patch on this thread, I think the pgbench -c 4 and -c 1 cases that I tested suggest it is a particular problem for parallel sorts, as there is a much bigger both absolute and proportional difference in transaction throughput between those two with the patch applied. It seems reasonable to suppose the difference would be larger still if we were considering a single parallel sort, as opposed to multiple independent sorts (of the same data) that happen to occur in parallel. I think that I may have been too optimistic when I said that there was an apparent trend of memory bandwidth per core merely stagnating: http://users.ece.cmu.edu/~omutlu/pub/mutlu_memory-scaling_imw13_invited-talk.pdf As slide 8 indicates, memory capacity per core is expected to go down 30% every two years, while the trend for memory bandwidth per core is even worse. -- 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] IMPORT FOREIGN SCHEMA statement
On Thu, Jul 10, 2014 at 9:52 AM, Tom Lane t...@sss.pgh.pa.us wrote: I wrote: I've started looking at this patch. I wonder whether it's really such a great idea to expect the FDW to return a list of parsetrees for CREATE FOREIGN TABLE commands; that seems like a recipe for breakage anytime we change the parsetree representation, say add a field to ColumnDef. The alternative I'm thinking about is to have the FDW pass back a list of strings, which would be textual CREATE FOREIGN TABLE commands. Here's a rather-heavily-editorialized draft patch that does it like that. This patch takes the viewpoint I espoused nearby of allowing names in the LIMIT TO clause that aren't present on the remote server. If we decide we want to hew to the letter of the standard on that, I'd be inclined to enforce this in the core code, not in individual FDWs as the submitted patch did. (I didn't much like that implementation anyway, since it didn't tell you which name it was unhappy about.) Woah, thanks. I've just been through this patch and it looks great. I guess that this implementation is enough as a first shot, particularly regarding the complexity that default expression import would bring up for postgres_fdw (point raised during the review, but not really discussed afterwards). Regards, -- Michael
Re: [HACKERS] IMPORT FOREIGN SCHEMA statement
Michael Paquier michael.paqu...@gmail.com writes: I guess that this implementation is enough as a first shot, particularly regarding the complexity that default expression import would bring up for postgres_fdw (point raised during the review, but not really discussed afterwards). Yeah. I'm actually more concerned about the lack of collation import, but that's unfixable unless we can figure out how to identify collations in a platform-independent way. 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] postgresql.auto.conf and reload
Josh Berkus j...@agliodbs.com writes: On 07/08/2014 10:07 AM, Robert Haas wrote: I haven't looked at the code in this area too carefully, but it seems to me like the flow ought to be: 1. Read all of the config files and determine what the final value present in each config file is. AFAICS we only care about the final value, not the final-value-in-each- config-file. 2. *Then*, in a second pass, enforce requirements like can't be changed except at server start. +1 This would also make conf.d much more useful; I wouldn't have to worry as much about overlapping config settings. Sounds like a 9.5 feature, though. No, ALTER SYSTEM is there now and it needs to work right in its first release. I will go fix this if nobody else does. 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] better atomics - v0.5
On Wed, Jul 9, 2014 at 8:58 PM, Robert Haas robertmh...@gmail.com wrote: On Tue, Jul 8, 2014 at 2:21 AM, Amit Kapila amit.kapil...@gmail.com wrote: On Thu, Jul 3, 2014 at 10:56 AM, Amit Kapila amit.kapil...@gmail.com wrote: Further review of patch: 1. /* * pg_atomic_test_and_set_flag - TAS() * * Acquire/read barrier semantics. */ STATIC_IF_INLINE_DECLARE bool pg_atomic_test_set_flag(volatile pg_atomic_flag *ptr); a. I think Acquire and read barrier semantics aren't equal. With acquire semantics, the results of the operation are available before the results of any operation that appears after it in code which means it applies for both load and stores. Definition of read barrier just ensures loads. So will be right to declare like above in comments? Yeah. Barriers can be by the operations they keep from being reordered (read, write, or both) and by the direction in which they prevent reordering (acquire, release, or both). So in theory there are 9 combinations: read barrier (both directions) read barrier (acquire) read barrier (release) write barrier (both directions) write barrier (acquire) write barrier (both directions) full barrier (both directions) full barrier (acquire) full barrier (release) With these definitions, I think it will fall into *full barrier (acquire)* category as it needs to ensure that no reordering should happen for both load and store. As an example, we can refer its implementation for msvc which ensures that it follows full memory barrier semantics. #define TAS(lock) (InterlockedCompareExchange(lock, 1, 0)) To make things more confusing, a read barrier plus a write barrier need not equal a full barrier. Read barriers prevent reads from being reordered with other reads, and write barriers keep writes from being reordered with other writes, but neither prevents a read from being reordered relative to a write. A full barrier, however, must prevent all reordering: read/read, read/write, write/read, and write/write. Clarity here is essential. barrier.h only proposed to implement the following: read barrier (both directions), write barrier (both directions), full barrier (both directions) As per my understanding of the general theory around barriers, read and write are defined to avoid reordering due to compiler and full memory barriers are defined to avoid reordering due to processors. There are some processors that support instructions for memory barriers with acquire, release and fence semantics. I think the current definitions of barriers is as per needs of usage in PostgreSQL and not by referring standard (am I missing something here), however as you explained the definitions are quite clear. The reason I did that is because it seemed to be more or less what Linux was doing, and it's generally suitable for lock-less algorithms. The acquire and release semantics come into play specifically when you're using barriers to implement locking primitives, which is isn't what I was trying to enable. Now by implementing atomic-ops, I think we are moving more towards lock-less algorithms, so I am not sure if we just want to adhere to existing definitions as you listed above (what current barrier.h implements) and implement accordingly or we can extend the existing definitions. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
Re: [HACKERS] 9.5: UPDATE/DELETE .. ORDER BY .. LIMIT ..
On Wed, Jul 9, 2014 at 8:42 PM, Marko Tiikkaja ma...@joh.to wrote: Hi Amit, On 5/14/14 6:41 AM, Amit Kapila wrote: IIUC, the way new design will work is that for new tuple we will now get tableoid+TID, modified column values as an input (for inheritance tables we will get this for all child tables as well) for ModifyTable and get old tuple (which in current case will be provided by MergeAppend or in general by some scan node) from some node beneath the ModifyTable. It then matches the tableoid from old tuple with appropriate tableoid incase of child tables and then form the new tuple for that tableoid using old tuple and modified column values. Having now read the discussion upthread a bit more carefully, I think one of us is confused. AIUI, what was suggested was that the plan nodes below the ModifyTable node would only give you back the modified columns, the tableoid and the TID of the tuple, and no old values at all. Plan node below ModifyTable will be a scan node, it will give you old tuple, whats the use of getting modified columns from it. We need modified columns for new tuple which can be input for ModifyTuple. In this case can we safely assume that we will always get tableoid from old tuple, ideally it should be there but just not sure It has to be there or otherwise the scheme won't work. Is there a specific case you're worried about? and another minor point is won't we get TID from old tuple (tuple we get from node beneath ModifyTable), what's the need to pass for new tuple? I don't understand this part, could you rephrase? Basically, I wanted to say that apart from modified columns, we just need to pass table OID. If I am reading correctly, the same is mentioned by Heikki as well. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
Re: [HACKERS] things I learned from working on memory allocation
On Tue, Jul 8, 2014 at 1:27 AM, Robert Haas robertmh...@gmail.com wrote: 6. In general, I'm worried that it's going to be hard to keep the overhead of parallel sort from leaking into the non-parallel case. With the no-allocator approach, every place that uses GetMemoryChunkSpace() or repalloc() or pfree() will have to handle the DSM and non-DSM cases differently, which isn't great for either performance or maintainability. And even with an allocator, the SortTuple array will need to use relative pointers in a DSM; that might burden the non-DSM case. I think you are right in saying that there can be a performance impact for non-parallel case due to pfree() (or other similar calls) and/or due to usage of relative pointers, but can it have measurable impact during actual sort operation? Changes for any of them doesn't seem to cause much impact for overall sort operation, although I am not sure what kind of impact usage of relative pointers can cause, do you any specific point in mind due to which you think that it can cause noticeable performance impact. If there is an noticeable impact, then do you think having separate file/infrastructure for parallel sort can help, basically non-parallel and parallel sort will have some common and some specific parts. The above layer will call the parallel/non-parallel functions based on operation need. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
Re: [HACKERS] psql: show only failed queries
On Wed, Jul 9, 2014 at 9:44 PM, Pavel Stehule pavel.steh...@gmail.com wrote: Barring any objection, I will commit this patch except tab-completion part. Committed. It can be a second discussion, but I am thinking so anywhere when we can use autocomplete, then we should it - Although it should not to substitute documentation, it is fast help about available options, mainly in situation where variables can hold a relative different content. I can prepare, if there will not any objection addition patch with complete autocomplete for all psql variables described in doc. I have no objection. But I found that the tab-completion for psql variables names are not complete. For example, COMP_KEYWORD_CASE is not displayed. Probably we should address this problem, too. 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
