Re: [HACKERS] Default Roles
* Heikki Linnakangas (hlinn...@iki.fi) wrote: On 05/13/2015 06:07 AM, Stephen Frost wrote: This does change the XLOG functions to require pg_monitor, as discussed on the other thread where it was pointed out by Heikki that the XLOG location information could be used to extract sensitive information based on what happens during compression. That seems like an orthogonal issue, not something that should be bundled in this patch. IIRC we didn't reach a consensus on what to do about the compression-leaks-information issue. One idea was to make it configurable on a per-table basis, and if we do that, perhaps we don't need to restrict access to pg_current_xlog_location() and friends. Alright, I'll pull it out. I see it's already been added to the open-items list, so we shouldn't forget about it. For my 2c, I'd much rather have the information restricted to a privileged role instead of having to disable the feature. Further, all tables need to be considered as having privileged information, not just systems ones like pg_authid, as the user might not have rights on the other columns or rows in the table. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] Let PostgreSQL's On Schedule checkpoint write buffer smooth spread cycle by tuning IsCheckpointOnSchedule?
(please keep the mailing list CC'd, and please don't top-post) On 05/13/2015 05:00 AM, digoal zhou wrote: I test it, but use exponent not very perfect in any environment. why cann't use time only? As you mentioned yourself earlier, if you only use time but you reach checkpoint_segments before checkpoint_timeout, you will not complete the checkpoint until you'd already need to begin the next checkpoint. You can't completely ignore checkpoint_segments. Comparing the numbers you give below with compensate-fpw-effect-on-checkpoint-scheduling-1.patch, with the ones from your first post, it looks like the patch already made the situation much better. You still have a significant burst in the beginning of the checkpoint cycle, but it's a lot smaller than without the patch. Before the patch, the count topped at 9078, and below it topped at 2964. There is a strange lull after the burst, I'm not sure what's going on there, but overall it seems like a big improvement. Did the patch alleviate the bump in latency that pgbench reports? I put the count numbers from your original post and below into a spreadsheet, and created some fancy charts. See attached. It shows the same thing but with pretty pictures. Assuming we want the checkpoint to be spread as evenly as possible across the cycle, the ideal would be a straight line from 0 to about 15 in 270 seconds in the cumulative chart. You didn't give the full data, but you can extrapolate the lines to get a rough picture of how close the different versions are from that ideal. In summary, the X^1.5 correction seems to work pretty well. It doesn't completely eliminate the problem, but it makes it a lot better. I don't want to over-compensate for the full-page-write effect either, because there are also applications where that effect isn't so big. For example, an application that performs a lot of updates, but all the updates are on a small number of pages, so the full-page-write storm immediately after checkpoint doesn't last long. A worst case for this patch would be such an application - lots of updates on only a few pages - with a long checkpoint_timeoout but relatively small checkpoint_segments, so that checkpoints are always driven by checkpoint_segments. I'd like to see some benchmarking of that worst case before committing anything like this. --end- checkpoint start buffer__sync__start num_buffers: 524288, dirty_buffers: 156931 r1_or_w2 2, pid: 29132, min: 44, max: 151, avg: 52, sum: 49387, count: 932 --end- r1_or_w2 2, pid: 29132, min: 44, max: 95, avg: 49, sum: 41532, count: 837 --end- r1_or_w2 2, pid: 29132, min: 44, max: 747, avg: 54, sum: 100419, count: 1849 --end- r1_or_w2 2, pid: 29132, min: 44, max: 372, avg: 52, sum: 110701, count: 2090 --end- r1_or_w2 2, pid: 29132, min: 44, max: 115, avg: 57, sum: 147510, count: 2575 --end- r1_or_w2 2, pid: 29132, min: 44, max: 470, avg: 58, sum: 145217, count: 2476 --end- r1_or_w2 2, pid: 29132, min: 44, max: 120, avg: 54, sum: 161401, count: 2964 --end- r1_or_w2 2, pid: 29132, min: 44, max: 208, avg: 59, sum: 170280, count: 2847 --end- r1_or_w2 2, pid: 29132, min: 44, max: 10089, avg: 62, sum: 136106, count: 2181 --end- r1_or_w2 2, pid: 29132, min: 41, max: 487, avg: 56, sum: 88990, count: 1570 --end- r1_or_w2 2, pid: 29132, min: 39, max: 102, avg: 55, sum: 59807, count: 1083 --end- r1_or_w2 2, pid: 29132, min: 40, max: 557, avg: 56, sum: 117274, count: 2083 --end- r1_or_w2 2, pid: 29132, min: 44, max: 537, avg: 58, sum: 169867, count: 2882 --end- r1_or_w2 2, pid: 29132, min: 44, max: 147, avg: 60, sum: 92835, count: 1538 --end- r1_or_w2 2, pid: 29132, min: 30, max: 93, avg: 55, sum: 14641, count: 264 --end- r1_or_w2 2, pid: 29132, min: 48, max: 92, avg: 56, sum: 11834, count: 210 --end- r1_or_w2 2, pid: 29132, min: 45, max: 91, avg: 56, sum: 9151, count: 162 --end- r1_or_w2 2, pid: 29132, min: 46, max: 92, avg:
Re: [HACKERS] multivariate statistics / patch v6
Hello, this might be somewhat out of place but strongly related
to this patch so I'll propose this here.
This is a proposal of new feature for this patch or asking for
your approval for my moving on this as a different (but very
close) project.
===
Attached is v6 of the multivariate stats, with a number of
improvements:
...
2) fix of pg_proc issues (reported by Jeff)
3) rebase to current master
Unfortunately, the v6 patch suffers some system oid conflicts
with recently added ones. And what more unfortunate for me is
that the code for functional dependencies looks undone:)
I mention this because I recently had a issue from strong
correlation between two columns in dbt3 benchmark. Two columns in
some table are in strong correlation but not in functional
dependencies, there are too many values and the distribution of
them is very uniform so MCV is no use for the table (histogram
has nothing to do with equal conditions). As the result, planner
estimates the number of rows largely wrong as expected especially
for joins.
I, then, had a try calculating the ratio between the product of
distinctness of every column and the distinctness of the set of
the columns, call it multivariate coefficient here, and found
that it looks greately useful for the small storage space, less
calculation, and simple code.
The attached first is a script to generate problematic tables.
And the second is a patch to make use of the mv coef on current
master. The patch is a very primitive POC so no syntactical
interfaces involved.
For the case of your first example,
=# create table t (a int, b int, c int);
=# insert into t (select a/1, a/1, a/1
from generate_series(0, 99) a);
=# analyze t;
=# explain analyze select * from t where a = 1 and b = 1 and c = 1;
Seq Scan on t (cost=0.00..22906.00 rows=1 width=12)
(actual time=3.878..250.628 rows=1 loops=1)
Make use of mv coefficient.
=# insert into pg_mvcoefficient values ('t'::regclass, 1, 2, 3, 0);
=# analyze t;
=# explain analyze select * from t where a = 1 and b = 1 and c = 1;
Seq Scan on t (cost=0.00..22906.00 rows=9221 width=12)
(actual time=3.740..242.330 rows=1 loops=1)
Row number estimation was largely improved.
Well, my example,
$ perl gentbl.pl 1 | psql postgres
$ psql postgres
=# explain analyze select * from t1 where a = 1 and b = 2501;
Seq Scan on t1 (cost=0.00..6216.00 rows=1 width=8)
(actual time=0.030..66.005 rows=8 loops=1)
=# explain analyze select * from t1 join t2 on (t1.a = t2.a and t1.b = t2.b);
Hash Join (cost=1177.00..11393.76 rows=76 width=16)
(actual time=29.811..322.271 rows=32 loops=1)
Too bad estimate for the join.
=# insert into pg_mvcoefficient values ('t1'::regclass, 1, 2, 0, 0);
=# analyze t1;
=# explain analyze select * from t1 where a = 1 and b = 2501;
Seq Scan on t1 (cost=0.00..6216.00 rows=8 width=8)
(actual time=0.032..104.144 rows=8 loops=1)
=# explain analyze select * from t1 join t2 on (t1.a = t2.a and t1.b = t2.b);
Hash Join (cost=1177.00..11393.76 rows=305652 width=16)
(actual time=40.642..325.679 rows=32 loops=1)
It gives almost correct estimations.
I think the result above shows that the multivariate coefficient
is significant to imporove estimates when correlated colums are
involved.
Would you consider this in your patch? Otherwise I move on this
as a different project from yours if you don't mind. Except user
interface won't conflict with yours, I suppose. But finally they
should need some labor of consolidation.
regards,
1) fix of the contrib compile-time errors (reported by Jeff)
2) fix of pg_proc issues (reported by Jeff)
3) rebase to current master
4) fix a bunch of issues in the previous patches, due to referencing
some parts too early (e.g. histograms in the first patch, etc.)
5) remove the explicit DELETEs from pg_mv_statistic (in the regression
tests), this is now handled automatically by DROP TABLE etc.
6) number of performance optimizations in selectivity estimations:
(a) minimize calls to get_oprrest, significantly reducing
syscache calls
(b) significant reduction of palloc overhead in deserialization of
MCV lists and histograms
(c) use more compact serialized representation of MCV lists and
histograms, often removing ~50% of the size
(d) use histograms with limited deserialization, which also allows
caching function calls
(e) modified histogram bucket partitioning, resulting in more even
bucket distribution (i.e. producing buckets with more equal
density and about equal size of each dimension)
7) add functions for listing MCV list items and histogram buckets:
- pg_mv_mcvlist_items(oid)
- pg_mv_histogram_buckets(oid, type)
This is quite useful when analyzing the MCV lists / histograms.
8)
Re: [HACKERS] Sequence Access Method WIP
On 13/05/15 12:56, Heikki Linnakangas wrote: On 05/13/2015 07:10 AM, Alvaro Herrera wrote: Heikki, do you have time to go through this at this point? I'm afraid I won't :-(. I did intend to, but looking at the calendar, I won't have the time to review this thoroughly enough to commit. Sorry. We discussed using a single amdata column vs. any number of am-specific columns. We settled on amdata, but I'm still not 100% convinced that's the best approach. Just as a data point, this removes the log_cnt field and moves it into amdata in a non-human-readable format. So for someone who only uses the local seqam, this just makes things slightly worse. For more complicated seqam's, it would be even more important to display the state in a human-readable format. Perhaps it's OK that each seqam provides its own functions or similar to do that, but I'd like to revisit that decision. I do think it's ok for seqam to provide functions that can give you the data in human readable form. I think main argument against custom human readable columns is that it will kill any possibility to have common storage for sequences. There is also additional complexity in the API required for that. The performance implications are probably small as one could still define opaque bytea column and store the data same way a now. I still don't like the serial_sequenceam GUC. Not sure what to do instead. Needs some thought. I think it would be ok if this issue was solved by follow-up patch at later time. -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Default Roles
On 05/13/2015 06:07 AM, Stephen Frost wrote: This does change the XLOG functions to require pg_monitor, as discussed on the other thread where it was pointed out by Heikki that the XLOG location information could be used to extract sensitive information based on what happens during compression. That seems like an orthogonal issue, not something that should be bundled in this patch. IIRC we didn't reach a consensus on what to do about the compression-leaks-information issue. One idea was to make it configurable on a per-table basis, and if we do that, perhaps we don't need to restrict access to pg_current_xlog_location() and friends. - Heikki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Sequence Access Method WIP
On 05/13/2015 07:10 AM, Alvaro Herrera wrote: Heikki, do you have time to go through this at this point? I'm afraid I won't :-(. I did intend to, but looking at the calendar, I won't have the time to review this thoroughly enough to commit. Sorry. I haven't looked at the CREATE/DROP ACCESS METHOD FOR SEQUENCE syntax patch at all yet. We discussed using a single amdata column vs. any number of am-specific columns. We settled on amdata, but I'm still not 100% convinced that's the best approach. Just as a data point, this removes the log_cnt field and moves it into amdata in a non-human-readable format. So for someone who only uses the local seqam, this just makes things slightly worse. For more complicated seqam's, it would be even more important to display the state in a human-readable format. Perhaps it's OK that each seqam provides its own functions or similar to do that, but I'd like to revisit that decision. I still don't like the serial_sequenceam GUC. Not sure what to do instead. Needs some thought. - Heikki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] EvalPlanQual behaves oddly for FDW queries involving system columns
On 2015/05/13 6:22, Tom Lane wrote: I wrote: I did a very basic update of your postgres_fdw patch to test this with, and attach that so that you don't have to repeat the effort. I'm not sure whether we want to try to convert that into something committable. I'm afraid that the extra round trips involved in doing row locking this way will be so expensive that no one really wants it for postgres_fdw. It's more credible that FDWs operating against local storage would have use for it. Of course, if we don't do that then we still have your original gripe about ctid not being correct in EvalPlanQual results. I poked at this a bit and it seems like we could arrange to pass ctid through even in the ROW_MARK_COPY case, if we define the t_ctid field of a composite Datum as being the thing to use. The attached WIP, mostly-comment-free patch seems to fix the original test case. It would likely result in ctid coming out as (0,0) not (4294967295,0) for FDWs that don't take any special steps to return a valid ctid, as a result of the fact that heap_form_tuple just leaves that field zeroed. +1 I did the same thing when creating the first version of the patch [1]. In addition, I made another change to ForeignNext so that the FDWs to get ctid coming out as the same value (0, 0) instead of (4294967295,0) before and after an EvalPlanQual recheck. But IIRC, I think both of them were rejected by you ... We could fix that by adding an ItemPointerSetInvalid((td-t_ctid)) call to heap_form_tuple, but I dunno if we want to add even a small number of cycles for this purpose to such a core function. I thought so too when creating the first version. Best regards, Etsuro Fujita [1] http://www.postgresql.org/message-id/54b7691b.5080...@lab.ntt.co.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] Sequence Access Method WIP
On 13 May 2015 at 11:56, Heikki Linnakangas hlinn...@iki.fi wrote: On 05/13/2015 07:10 AM, Alvaro Herrera wrote: Heikki, do you have time to go through this at this point? I'm afraid I won't :-(. I did intend to, but looking at the calendar, I won't have the time to review this thoroughly enough to commit. Sorry. I haven't looked at the CREATE/DROP ACCESS METHOD FOR SEQUENCE syntax patch at all yet. We discussed using a single amdata column vs. any number of am-specific columns. We settled on amdata, but I'm still not 100% convinced that's the best approach. Just as a data point, this removes the log_cnt field and moves it into amdata in a non-human-readable format. So for someone who only uses the local seqam, this just makes things slightly worse. For more complicated seqam's, it would be even more important to display the state in a human-readable format. Perhaps it's OK that each seqam provides its own functions or similar to do that, but I'd like to revisit that decision. I still don't like the serial_sequenceam GUC. Not sure what to do instead. Needs some thought. This has had around 2 years of thought at this point. I don't agree it needs more thought. There is one clear use case for this and it is of benefit to many distributed architectures. I don't see what calamity will occur if we commit this. If you don't want a sequence AM, don't ever use this. -- Simon Riggshttp://www.2ndQuadrant.com/ http://www.2ndquadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training Services
Re: [HACKERS] Streaming replication and WAL archive interactions
On Mon, May 11, 2015 at 12:00 PM, Heikki Linnakangas hlinn...@iki.fi wrote: And here is a new version of the patch. I kept the approach of using pgstat, but it now only polls pgstat every 10 seconds, and doesn't block to wait for updated stats. It's not entirely a new problem, but this error message has gotten pretty crazy: + (errmsg(WAL archival (archive_mode=on/always/shared) requires wal_level \archive\, \hot_standby\, or \logical\))); Maybe: WAL archival cannot be enabled when wal_level is minimal I think the documentation should be explicit about what happens if the primary archives a file and dies before the standby gets notified that the archiving happened. The standby, running in shared mode, is then promoted. My first guess would be that the standby will end up with files that thinks it needs to archive but, being unable to do so because they're already there, they'll live forever in pg_xlog. I hope that's not the case. -- 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] Streaming replication and WAL archive interactions
On Wed, May 13, 2015 at 8:53 AM, Heikki Linnakangas hlinn...@iki.fi wrote: Our manual says that archive_command should refuse to overwrite an existing file. But to work-around the double-archival problem, where the same file is archived twice, it would be even better if it would simply return success if the file exists, *and has identical contents*. I don't know how to code that logic in a simple one-liner though. This is why we really, really need that pg_copy command that was proposed a while back. -- 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] RFC: Non-user-resettable SET SESSION AUTHORISATION
* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: Craig Ringer wrote: For some time I've wanted a way to SET SESSION AUTHORISATION or SET ROLE in a way that cannot simply be RESET, so that a connection may be handed to a less-trusted service or application to do some work with. Some years back, I checked the SQL standard for insight on how they handle this stuff (courtesy of Jim Nasby IIRC). It took me a while to figure out that the way they do it is not to have a RESET command in the first place! In their model, you enter a secure execution context (for example, an SQL function) by calling SET SESSION AUTHORIZATION; and once there, the only way to revert to the original session authorization is to exit the execution context -- and once that happens, the attacker no longer has control. Since they have reduced privileges, they can't call SET SESSION AUTHORIZATION themselves to elevate their access. In this model, you're automatically protected. I mentioned this in some developer meeting; got blank stares back, IIRC. I mentioned it to Stephen in hallway track, and as I recall he was in agreement with what I was proposing. Biggest problem is, I can't recall in detail what it was. The issue here ends up being that you don't get the pooling advantage because the connection pooler ends up having to drop the connection after using it. I'm not against a 'SET-and-never-return' concept, but I don't think it'd help what Craig's after. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] One question about security label command
2015-05-13 21:45 GMT+09:00 Robert Haas robertmh...@gmail.com: On Sun, May 10, 2015 at 3:15 AM, Kohei KaiGai kai...@kaigai.gr.jp wrote: 2015-05-01 9:52 GMT+09:00 Kohei KaiGai kai...@kaigai.gr.jp: 2015-05-01 7:40 GMT+09:00 Alvaro Herrera alvhe...@2ndquadrant.com: Kouhei Kaigai wrote: * Tom Lane (t...@sss.pgh.pa.us) wrote: The idea of making the regression test entirely independent of the system's policy would presumably solve this problem, so I'd kind of like to see progress on that front. Apologies, I guess it wasn't clear, but that's what I was intending to advocate. OK, I'll try to design a new regression test policy that is independent from the system's policy assumption, like unconfined domain. Please give me time for this work. Any progress here? Not done. The last version I rebuild had a trouble on user/role transition from unconfined_u/unconfined_r to the self defined user/role... So, I'm trying to keep the user/role field (that is not redefined for several years) but to define self domain/types (that have been redefined multiple times) for the regression test at this moment. The second approach above works. I defined a own privileged domain (sepgsql_regtest_superuser_t) instead of system's unconfined_t domain. The reason why regression test gets failed was, definition of unconfined_t in the system default policy was changed to bypass multi-category rules; which our regression test depends on. So, the new sepgsql_regtest_superuser_t domain performs almost like as unconfined_t, but restricted by multi-category policy as traditional unconfined_t did. It is self defined domain, so will not affected by system policy change. Even though the sepgsql-regtest.te still uses unconfined_u and unconfined_r pair for selinux-user and role, it requires users to define additional selinux-user by hand if we try to define own one. In addition, its definition has not been changed for several years. So, I thought it has less risk to rely on unconfined_u/unconfined_r field unlike unconfined_t domain. Can you add this to the next CommitFest? OK, done https://commitfest.postgresql.org/5/249/ -- 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] Auditing extension for PostgreSQL (Take 2)
On Mon, May 11, 2015 at 9:07 PM, David Steele da...@pgmasters.net wrote:
On 5/1/15 5:58 AM, Sawada Masahiko wrote:
For now, since pg_audit patch has a pg_audit_ddl_command_end()
function which uses part of un-committed deparsing utility commands
patch API,
pg_audit patch is completely depend on that patch.
If API name, interface are changed, it would affect for pg_audit patch.
I'm not sure about progress of deparsing utility command patch but
you have any idea if that patch is not committed into 9.5 until May
15?
The attached v12 patch removes the code that became redundant with
Alvaro committing the event trigger/deparse work. I've updated the
regression tests to reflect the changes, which were fairly minor and add
additional information to the output. There are no longer any #ifdef'd
code blocks.
This is not a full review, but just a few thoughts...
What happens if the server crashes? Presumably, audit records emitted
just before the crash can be lost, possibly even if the transaction
went on to commit. That's no worse than what is already the case for
regular logging, of course, but it's maybe a bit more relevant here
because of the intended use of this information.
+if (audit_on_relation(relOid, auditOid, auditPerms))
+{
+auditEventStack-auditEvent.granted = true;
+}
Braces around single-statement blocks are not PostgreSQL style.
I wonder if driving the auditing system off of the required
permissions is really going to work well. That means that decisions
we make about permissions enforcement will have knock-on effects on
auditing. For example, the fact that we check permission on a view
rather than on the underlying tables will (I think) flow through to
how the auditing happens.
+stackItem-auditEvent.commandTag == T_DoStmt
Use IsA(..., DoStmt) for this kind of test. There are many instances
of this pattern in the patch; they should al be fixed.
Using auditLogNotice to switch the log level between LOG and NOTICE is
weird. Switching from LOG to NOTICE is an increase in the logging
level relative to client_min_messages, but a decrease relative to
log_min_messages. With default settings, logging at LOG will go only
to the log (not the client) and logging at NOTICE will go only to the
client (not the log).
The documentation and comments in this patch are, by my estimation,
somewhat below our usual standard. For example:
+ DefineCustomBoolVariable(pg_audit.log_notice,
+Raise a
notice when logging,
Granted, there is a fuller explanation in the documentation, but that
doesn't make that explanation particularly good. (One might also
wonder why the log level isn't fully configurable instead of offering
only two choices.)
Here's another example:
+ DefineCustomBoolVariable(pg_audit.log_parameter,
+Enable
statement parameter logging,
It would be far better to say lnclude the values of statement
parameters in auditing messages or something like that. It is quite
clear that this GUC doesn't enable some sort of general statement
parameter logging facility; it changes the contents of auditing
messages that would have been emitted either way.
I don't want to focus too much on this particular example. The
comments and documentation really deserve a bit more attention
generally than they have gotten thus far. I am not saying they are
bad. I am just saying that, IMHO, they are not as good as we
typically try to make them.
--
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] Sequence Access Method WIP
On 13/05/15 14:15, Heikki Linnakangas wrote: I don't see what calamity will occur if we commit this. If you don't want a sequence AM, don't ever use this. I'd like the API to be good for its purpose. Also, I did mention that the the current patch makes the situation slightly worse for people who never use this: it makes the log_cnt field non human-readable. That's a really minor thing, but it shows that it *does* matter how this is implemented, even if you only ever use the local seq AM. It definitely does matter. I don't think we'll find perfect compromise here though, you can either do it one way or the other. Trust me it does not make me happy either, I like perfect solutions too, but when there is lack of perfect solution I prefer the simpler one. Both of the solutions have drawbacks - amdata has opaque blob which does not store data in user visible way but that can be worked around by providing function that shows it in human readable way (and the dump function for many sequence types actually does that). - multiple columns basically kill any future ability to unify the storage for sequences and also adds complexity, especially around alter table (since it means drop/add column and stuff) But I already wrote both versions anyway so from that perspective it does not matter much which part we merge. (As a side-node I would have preferred to have this discussion earlier than 2 days before feature freeze because the current implementation is something that we agreed on several months ago so there was plenty of time to revisit that decision.) -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Disabling trust/ident authentication configure option
On Mon, May 11, 2015 at 10:00 PM, Robert Haas robertmh...@gmail.com wrote: On Thu, May 7, 2015 at 4:57 PM, Stephen Frost sfr...@snowman.net wrote: * Robert Haas (robertmh...@gmail.com) wrote: On Thu, May 7, 2015 at 11:02 AM, Stephen Frost sfr...@snowman.net wrote: I realize it's not going to be popular, but I'd love to have 'trust' only allowed if a command-line option is passed to the postmaster or something along those lines. It's really got no business being an option for a network service like PG. I disagree wholeheartedly. There is such a thing as a trusted network. Likely a good topic of conversation to be had in Ottawa. :) I agree that there are trusted networks, but the ones that I work with still expect network services to require authentication and authorization. Perhaps they're not really trusted then, from your perspective. On the other hand, I suppose if you use pg_hba to limit which accounts can be logged into with 'trust' then you might be able to have, say, a read-only user/database that anyone could see. That's a pretty narrow case though and I'd rather we figure out how to address it directly and more specifically (no-password login roles?) than the broad disable-all-authentication trust method. Let's suppose that you have an application server and a DB server running on the same node. That turns out to be too much load, so you move the application server to a separate machine and connect the two machines with a crossover cable, or a VLAN that has nothing else on it. To me, it's quite sane to want connections on that network to proceed without authentication or authorization. If you've got to open up the database more than that then, yes, you need authentication and authorization. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company Even in this case it still means that any breach in any of the network services running on your application server would immediately own your database, or at least everything your application can access. This applies even to totally unrelated services running with restricted permissions. Using password or certificate based authentication at least gives you the additional security of local filesystem access controls and is not much harder to setup. M2M authentication is always a difficult topic as the authentication tokens have to be secured but I would agree that a more specific / secure method than disable-all-authentication would be preferable. Best regards, Volker
Re: [HACKERS] Sequence Access Method WIP
On 05/13/2015 02:12 PM, Simon Riggs wrote: This has had around 2 years of thought at this point. I don't agree it needs more thought. Noted. There is one clear use case for this and it is of benefit to many distributed architectures. Right. What's your point? I don't see what calamity will occur if we commit this. If you don't want a sequence AM, don't ever use this. I'd like the API to be good for its purpose. Also, I did mention that the the current patch makes the situation slightly worse for people who never use this: it makes the log_cnt field non human-readable. That's a really minor thing, but it shows that it *does* matter how this is implemented, even if you only ever use the local seq AM. - Heikki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Streaming replication and WAL archive interactions
On 05/13/2015 03:36 PM, Robert Haas wrote: On Mon, May 11, 2015 at 12:00 PM, Heikki Linnakangas hlinn...@iki.fi wrote: And here is a new version of the patch. I kept the approach of using pgstat, but it now only polls pgstat every 10 seconds, and doesn't block to wait for updated stats. It's not entirely a new problem, but this error message has gotten pretty crazy: + (errmsg(WAL archival (archive_mode=on/always/shared) requires wal_level \archive\, \hot_standby\, or \logical\))); Maybe: WAL archival cannot be enabled when wal_level is minimal I think the documentation should be explicit about what happens if the primary archives a file and dies before the standby gets notified that the archiving happened. Yes, good point. The standby, running in shared mode, is then promoted. My first guess would be that the standby will end up with files that thinks it needs to archive but, being unable to do so because they're already there, they'll live forever in pg_xlog. I hope that's not the case. Hmm. That is exactly what happens. The standby will attempt to archive them, which will fail, so the archiver will get stuck retrying. That's not actually a new problem though. Even with a single server doing archiving, it's possible that you crash just after archive_command has archived a file, but before it has created the .done file. After restart, the server will try to archive the file again, which will fail. But yeah, with this patch, that's much more likely to happen after a promotion. Our manual says that archive_command should refuse to overwrite an existing file. But to work-around the double-archival problem, where the same file is archived twice, it would be even better if it would simply return success if the file exists, *and has identical contents*. I don't know how to code that logic in a simple one-liner though. - Heikki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Why does contain_leaked_vars believe MinMaxExpr is safe?
On Tue, May 12, 2015 at 7:22 PM, Tom Lane t...@sss.pgh.pa.us wrote: MinMaxExpr is an implicit invocation of a btree comparison function. Are we supposing that all of those are necessarily leakproof? I suspect it's an oversight, because the comment gives no hint that any such intention was present. It's been more than three years since I committed that code (under a different function name) so my memory is a little fuzzy, but I believe it just didn't occur to me that MinMaxExpr could include a function call. I suspect it's safe in practice, but in theory it's probably a bug. -- 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] Disabling trust/ident authentication configure option
On Wed, May 13, 2015 at 8:01 AM, Volker Aßmann volker.assm...@gmail.com wrote: Even in this case it still means that any breach in any of the network services running on your application server would immediately own your database, or at least everything your application can access. This applies even to totally unrelated services running with restricted permissions. Using password or certificate based authentication at least gives you the additional security of local filesystem access controls and is not much harder to setup. M2M authentication is always a difficult topic as the authentication tokens have to be secured but I would agree that a more specific / secure method than disable-all-authentication would be preferable. Sure, opinions on the best way to do any given thing are going to vary, and nobody's trying to prevent you from configuring your instances of PostgreSQL however you like. The email to which I was responding was suggesting limiting MY ability to set up MY instances of PostgreSQL the way I like. And I'm opposed to that. All of this is fairly far afield from the original topic of this thread, which was whether a configure option disabling trust + ident authentication would be a good idea. I said no. Then we had a bunch of counter-proposals: Alvaro: Support a configure switch whose value is a comma-separated list of authentication methods to disable. Peter: Generalized hardening facility. Andrew: Like what Alvaro said, but require at least one of trust + peer to remain enabled so people can't hose themselves. Andrew, v2: Rip out RFC1413 ident authentication completely. Stephen: Require a command-line option to use trust auth. There's clearly no consensus on any of these proposals, and most of them don't address your original requirement anyway, though Alvaro's would. I guess the point is that nothing is going to get changed here on one person's say-so if other people don't agree, so if you want to get something done, you're going to need to pick something that can achieve consensus and then implement that. Also, anything you want to get done is certainly going to be in 9.6 at the earliest, because the time for 9.5 proposals has already come and gone. -- 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] Default Roles
All,
* Heikki Linnakangas (hlinn...@iki.fi) wrote:
On 05/13/2015 06:07 AM, Stephen Frost wrote:
This does change the XLOG functions to require pg_monitor, as discussed
on the other thread where it was pointed out by Heikki that the XLOG
location information could be used to extract sensitive information
based on what happens during compression.
That seems like an orthogonal issue, not something that should be
bundled in this patch. IIRC we didn't reach a consensus on what to
do about the compression-leaks-information issue. One idea was to
make it configurable on a per-table basis, and if we do that,
perhaps we don't need to restrict access to
pg_current_xlog_location() and friends.
Updated patch attached which removes the changes to the XLOG location
functions and adds checks for AlterRole and RenameRole to prevent
altering or renaming the default roles. Also adds '\duS'/'\dgS'
support to psql, to show default roles only when asked.
Thanks!
Stephen
From 88615d712892220cfe4c338317842e368f1fb62e Mon Sep 17 00:00:00 2001
From: Stephen Frost sfr...@snowman.net
Date: Thu, 7 May 2015 23:35:03 -0400
Subject: [PATCH] Create default roles for administrative functions
To reduce the number of users on a system who are superusers,
create a set of roles by default during initdb which are allowed to run
certain functions that allow non-superusers to perform specific
administrative tasks and have access to privileged information.
The prefix pg_ is reserved for default system roles, similar to
schemas and tablespaces. pg_upgrade is modified to check for any roles
which start with pg_ and complain if they exist. pg_dumpall is
modified to not dump out roles starting with pg_ on 9.5-and-above
systems. CreateRole is modified to refuse creation of roles which start
with pg_, similar to CreateSchema. psql only shows default roles when
called with \duS or \dgS, similar to other system objects.
Roles created are: pg_backup, pg_monitor, pg_replay, pg_replication,
pg_rotate_logfile, pg_signal_backend, pg_file_settings and pg_admin.
---
contrib/test_decoding/expected/permissions.out | 8 +--
doc/src/sgml/catalogs.sgml | 9 +++
doc/src/sgml/user-manag.sgml | 91 ++
src/backend/access/transam/xlogfuncs.c | 30 +
src/backend/catalog/catalog.c | 5 +-
src/backend/catalog/system_views.sql | 6 +-
src/backend/commands/user.c| 30 +++--
src/backend/replication/logical/logicalfuncs.c | 17 ++---
src/backend/replication/slotfuncs.c| 29
src/backend/replication/walsender.c| 8 ++-
src/backend/utils/adt/misc.c | 12 ++--
src/backend/utils/adt/pgstatfuncs.c| 25 ---
src/backend/utils/misc/guc.c | 7 ++
src/bin/pg_dump/pg_dumpall.c | 2 +
src/bin/pg_upgrade/check.c | 40 ++-
src/bin/psql/command.c | 4 +-
src/bin/psql/describe.c| 5 +-
src/bin/psql/describe.h| 2 +-
src/bin/psql/help.c| 4 +-
src/include/catalog/pg_authid.h| 19 +-
20 files changed, 280 insertions(+), 73 deletions(-)
diff --git a/contrib/test_decoding/expected/permissions.out b/contrib/test_decoding/expected/permissions.out
index 212fd1d..79a7f86 100644
--- a/contrib/test_decoding/expected/permissions.out
+++ b/contrib/test_decoding/expected/permissions.out
@@ -54,13 +54,13 @@ RESET ROLE;
-- plain user *can't* can control replication
SET ROLE lr_normal;
SELECT 'init' FROM pg_create_logical_replication_slot('regression_slot', 'test_decoding');
-ERROR: must be superuser or replication role to use replication slots
+ERROR: must be superuser or member of pg_replication to use replication slots
INSERT INTO lr_test VALUES('lr_superuser_init');
ERROR: permission denied for relation lr_test
SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1');
-ERROR: must be superuser or replication role to use replication slots
+ERROR: must be superuser or member of pg_replication to use replication slots
SELECT pg_drop_replication_slot('regression_slot');
-ERROR: must be superuser or replication role to use replication slots
+ERROR: must be superuser or member of pg_replication to use replication slots
RESET ROLE;
-- replication users can drop superuser created slots
SET ROLE lr_superuser;
@@ -90,7 +90,7 @@ SELECT 'init' FROM pg_create_logical_replication_slot('regression_slot', 'test_d
RESET ROLE;
SET ROLE lr_normal;
SELECT pg_drop_replication_slot('regression_slot');
-ERROR: must be superuser or replication role to use replication slots
+ERROR: must be superuser or member of pg_replication to use replication slots
RESET ROLE;
-- all users can see existing slots
SET ROLE
Re: [HACKERS] One question about security label command
On Sun, May 10, 2015 at 3:15 AM, Kohei KaiGai kai...@kaigai.gr.jp wrote: 2015-05-01 9:52 GMT+09:00 Kohei KaiGai kai...@kaigai.gr.jp: 2015-05-01 7:40 GMT+09:00 Alvaro Herrera alvhe...@2ndquadrant.com: Kouhei Kaigai wrote: * Tom Lane (t...@sss.pgh.pa.us) wrote: The idea of making the regression test entirely independent of the system's policy would presumably solve this problem, so I'd kind of like to see progress on that front. Apologies, I guess it wasn't clear, but that's what I was intending to advocate. OK, I'll try to design a new regression test policy that is independent from the system's policy assumption, like unconfined domain. Please give me time for this work. Any progress here? Not done. The last version I rebuild had a trouble on user/role transition from unconfined_u/unconfined_r to the self defined user/role... So, I'm trying to keep the user/role field (that is not redefined for several years) but to define self domain/types (that have been redefined multiple times) for the regression test at this moment. The second approach above works. I defined a own privileged domain (sepgsql_regtest_superuser_t) instead of system's unconfined_t domain. The reason why regression test gets failed was, definition of unconfined_t in the system default policy was changed to bypass multi-category rules; which our regression test depends on. So, the new sepgsql_regtest_superuser_t domain performs almost like as unconfined_t, but restricted by multi-category policy as traditional unconfined_t did. It is self defined domain, so will not affected by system policy change. Even though the sepgsql-regtest.te still uses unconfined_u and unconfined_r pair for selinux-user and role, it requires users to define additional selinux-user by hand if we try to define own one. In addition, its definition has not been changed for several years. So, I thought it has less risk to rely on unconfined_u/unconfined_r field unlike unconfined_t domain. Can you add this to the next CommitFest? -- 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] Silent Data Manipulation - Time(stamp) w/o Time Zone Literals w/ TZ
On Tue, May 12, 2015 at 1:57 PM, David G. Johnston david.g.johns...@gmail.com wrote: In a literal that has been determined to be timestamp without time zone, PostgreSQL will silently ignore any time zone indication. That is, the resulting value is derived from the date/time fields in the input value, and is not adjusted for time zone. http://www.postgresql.org/docs/9.3/static/datatype-datetime.html 8.5.1.3 Time Stamps Not exactly a widely impactful decision but is there harm in emitting a NOTICE: timezone specification ignored Well, it could emit a catastrophic amount of log chatter. Try doing a bulk load into a table with a column of that type. In general, I'm not a big fan of accepting things and ignoring them. We've prided ourselves on having tighter data type checking than pretty much any other database product out there, but somehow we still have edge cases like this floating around. -- 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] subxcnt defined as signed integer in SnapshotData and SerializeSnapshotData
On Sat, May 9, 2015 at 8:48 AM, Michael Paquier michael.paqu...@gmail.com wrote: On Fri, May 8, 2015 at 10:27 PM, Simon Riggs wrote: On 8 May 2015 at 13:02, Michael Paquier wrote: I think that we should redefine subxcnt as uint32 for consistency with xcnt, and remove the two assertions that 924bcf4 has introduced. I could get a patch quickly done FWIW. (uint32) +1 Attached is the patch. This has finished by being far simpler than what I thought first. I'm just going to remove the useless assertion for now. What you're proposing here may (or may not) be worth doing, but it carries a non-zero risk of breaking something somewhere, if anyone is relying on the signed-ness of that type. Removing the assertion is definitely safe. -- 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] Default Roles (was: Additional role attributes)
On Wed, May 13, 2015 at 10:16:39AM -0400, Robert Haas wrote: On Tue, May 12, 2015 at 11:07 PM, Stephen Frost sfr...@snowman.net wrote: Thoughts? Comments? Suggestions? Yes: let's punt this to 9.6. The decisions you're making here are way too significant to be making a couple of days before feature freeze, and this patch has changed massively since it was first submitted. There isn't time now for people who want to have an opinion on this to form an educated one. Yeah, pretty much any patch that needs significant redesign at this point should be kept for 9.6. -- 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] a few thoughts on the schedule
On Wed, May 13, 2015 at 11:09 AM, Tom Lane t...@sss.pgh.pa.us wrote: - Code review for foreign/custom join pushdown patch - Whacks around my earlier commit without agreement or adequate discussion, apparently on the theory that Tom always knows best. As far as that goes, obviously I've got strong opinions on what the FDW APIs ought to look like, and I'm happy to discuss this issue further --- but I think that's a topic for the mop-up period, not for this week. What I think we need to be doing this week is triage. Commit what's ready, punt what's not. I'll post a separate list about that. That sounds like a fine plan. Thanks for responding. -- 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] PATCH: pgbench allow '=' in \set
On Thu, May 7, 2015 at 2:18 PM, Fabien COELHO coe...@cri.ensmp.fr wrote: Since an expression syntax has been added to pgbench, I find that the readability of expressions is not great. An extra '=' improves the situation for me: \set id = 1 + abs((:id * 1021) % (10 * :scale)) seems slightly better than: \set id 1 + abs((:id * 1021) % (10 * :scale)) But that is debatable! I would like to debate that. :-) Generally, I don't like optional noise words. A good example of how this can bite you is the whole problem with LOCK TABLE foo. You can also write that as LOCK foo. What if you want to lock something that's not a table? If the word table were required, then you could support LOCK VIEW foo or LOCK MATERIALIZED VIEW foo or even LOCK FUNCTION foo(). But because the word TABLE is optional, that syntax creates parser ambiguities. We have not been able to agree on a reasonable solution to this problem, and it has blocked implementation of this important and useful feature for years. While I can't foresee what trouble specifically your proposal might cause, I have become wary of such things. I also don't like to restate what has already been said. \set at the beginning of the line tells you that you will be setting a variable. Adding = or := only restates the same thing. I agree it superficially looks a little nicer, but I'm not sure it's really going to add clarity, because it's basically just redundant. -- 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] a few thoughts on the schedule
Robert Haas robertmh...@gmail.com writes: It's pretty clear that the impending feature freeze proposed by core has spurred a lot of activity. This is both good and bad. On the good side, we're getting a bunch of stuff done. On the bad side, there is inevitably going to be a temptation to rush things in that are really not quite fully-baked just yet. If we fail to resist that temptation, we WILL pay the consequences. Yeah. At this point I think it's clear that only a minority of the remaining commitfest items can get into 9.5; we're going to have to punt many of them to next time. I'll post some thoughts about specific patches separately, but let's keep this thread for discussion of the overall situation. I am already concerned about some of the commits that have gone in very recently, particularly these: There is going to need to be a mop-up period, and we ought to be willing to revert anything we feel wasn't really ready. I don't feel that those decisions need to be made in a hurry though. I'm envisioning taking about a month to look more closely at committed patches and see what needs to be cleaned up or undone altogether. - Code review for foreign/custom join pushdown patch - Whacks around my earlier commit without agreement or adequate discussion, apparently on the theory that Tom always knows best. As far as that goes, obviously I've got strong opinions on what the FDW APIs ought to look like, and I'm happy to discuss this issue further --- but I think that's a topic for the mop-up period, not for this week. What I think we need to be doing this week is triage. Commit what's ready, punt what's not. I'll post a separate list about that. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Default Roles (was: Additional role attributes)
On Tue, May 12, 2015 at 11:07 PM, Stephen Frost sfr...@snowman.net wrote: Thoughts? Comments? Suggestions? Yes: let's punt this to 9.6. The decisions you're making here are way too significant to be making a couple of days before feature freeze, and this patch has changed massively since it was first submitted. There isn't time now for people who want to have an opinion on this to form an educated one. -- 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
[HACKERS] a few thoughts on the schedule
Hi, It's pretty clear that the impending feature freeze proposed by core has spurred a lot of activity. This is both good and bad. On the good side, we're getting a bunch of stuff done. On the bad side, there is inevitably going to be a temptation to rush things in that are really not quite fully-baked just yet. If we fail to resist that temptation, we WILL pay the consequences. Those consequences may include overt bugs (as per the recent discussion on what went wrong with multi-xacts) or bad design decisions from which we will not easily be able to disentangle ourselves. I am already concerned about some of the commits that have gone in very recently, particularly these: - Map basebackup tablespaces using a tablespace_map file - Discussion on pgsql-committers suggests that this may have some scary problems. Maybe I just don't understand the situation. - Allow on-the-fly capture of DDL event details - This looks really half-baked to me. At the least, the documentation expresses an optimism about what can be done with a pg_ddl_command that isn't realized by the code. - Code review for foreign/custom join pushdown patch - Whacks around my earlier commit without agreement or adequate discussion, apparently on the theory that Tom always knows best. - Add support for INSERT ... ON CONFLICT DO NOTHING/UPDATE - Huge feature introducing novel infrastructure. We also need to start thinking about what happens after feature freeze. The CommitFest application currently lists a 2015-06 CommitFest which, according to previous practice, would be expected to start on the 15th of the month. Given where we are now, that seems entirely unrealistic. I am doubtful that we will be ready to ship a beta by mid-June, let alone begin a new development cycle. -- 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
[HACKERS] Re: Abbreviated keys for Datum tuplesort (was: Re: B-Tree support function number 3 (strxfrm() optimization))
On Fri, Jan 23, 2015 at 4:13 PM, Andrew Gierth and...@tao11.riddles.org.uk wrote: [pruning the Cc: list and starting a new thread] Here's the cleaned-up version of the patch to allow abbreviated keys when sorting a single Datum. This also removes comments that suggest that the caller of tuplesort_begin_datum should ever have to care about the abbreviated key optimization. I'll add this to the CF. Committed. Thanks for the patch and your patience. -- 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
[HACKERS] Re: Abbreviated keys for Datum tuplesort (was: Re: B-Tree support function number 3 (strxfrm() optimization))
On Wed, May 13, 2015 at 11:40 AM, Robert Haas robertmh...@gmail.com wrote: Committed. Thanks for the patch and your patience. This comment was not updated: /* * The sortKeys variable is used by every case other than the datum and * hash index cases; it is set by tuplesort_begin_xxx. tupDesc is only * used by the MinimalTuple and CLUSTER routines, though. */ TupleDesc tupDesc; SortSupport sortKeys; /* array of length nKeys */ -- 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] KNN-GiST with recheck
On Wed, May 13, 2015 at 10:16 PM, Heikki Linnakangas hlinn...@iki.fi wrote: On 04/17/2015 12:05 PM, Alexander Korotkov wrote: On Wed, Feb 25, 2015 at 12:15 PM, Alexander Korotkov aekorot...@gmail.com wrote: Hi! On Tue, Feb 24, 2015 at 5:39 PM, Tomas Vondra tomas.von...@2ndquadrant.com wrote: On 17.2.2015 14:21, Alexander Korotkov wrote: On Sun, Feb 15, 2015 at 2:08 PM, Alexander Korotkov aekorot...@gmail.com mailto:aekorot...@gmail.com wrote: Revised patch with reordering in GiST is attached (knn-gist-recheck-in-gist.patch) as well as testing script (test.py). I meant to do a bit of testing on this (assuming it's still needed), but the patches need rebasing - Heikki fixed a few issues, so they don't apply cleanly. Both patches are revised. Both patches are rebased against current master. This looks pretty much ready. I'm going to spend some time on this on Friday, and if all looks good, commit. (Thursday's a public holiday here). Very good, thanks! One quick comment: It would be good to avoid the extra comparisons of the distances, when the index doesn't return any lossy items. As the patch stands, it adds one extra copyDistances() call and a cmp_distances() call for each tuple (in a knn-search), even if there are no lossy tuples. I will fix it until Friday. -- With best regards, Alexander Korotkov.
Re: [HACKERS] Proposal : REINDEX xxx VERBOSE
Robert Haas wrote: On Wed, May 13, 2015 at 2:10 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Uh, are we really using INFO to log this? I thought that was a discouraged level to use anymore. Why not NOTICE? Well, as Masahiko-san points out, VACUUM uses INFO. I can't see any good reason to make this different. I was misremembering the INFO situation. Here's one item in the archives I found in a very quick search, which says that INFO is the right thing to use: http://www.postgresql.org/message-id/24874.1231183...@sss.pgh.pa.us Cheers -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: [COMMITTERS] pgsql: Map basebackup tablespaces using a tablespace_map file
[redirecting to -hackers]
On 05/12/2015 01:30 PM, Amit Kapila wrote:
On Tue, May 12, 2015 at 9:02 PM, Andrew Dunstan and...@dunslane.net
mailto:and...@dunslane.net wrote:
On 05/12/2015 10:33 AM, Heikki Linnakangas wrote:
On 05/12/2015 04:42 PM, Andrew Dunstan wrote:
+
+ /*
+* Remove the existing symlink if any and
Create the symlink
+* under PGDATA. We need to use rmtree
instead of rmdir as
+* the link location might contain
directories or files
+* corresponding to the actual path. Some
tar utilities do
+* things that way while extracting symlinks.
+*/
+ if (lstat(linkloc, st) == 0
S_ISDIR(st.st_mode))
+ {
+ if (!rmtree(linkloc,true))
+ ereport(ERROR,
+ (errcode_for_file_access(),
+errmsg(could not remove
directory \%s\: %m,
+ linkloc)));
+ }
+ else
+ {
+ if (unlink(linkloc) 0 errno !=
ENOENT)
+ ereport(ERROR,
+ (errcode_for_file_access(),
+errmsg(could not remove
symbolic link \%s\: %m,
+ linkloc)));
+ }
+
That's scary. What tar utilitiy replaces the symlink with a
non-empty directory?
IIRC, it was star utility by using --copysymlinks options.
It will actually copy the symlink data at appropriate location,
but will not maintain symlink after extraction.
I don't have a link handly for it, but can again search for
it and send you if you want to have a look.
What if you call pg_start_backup() and take the backup with a
utility that follows symlinks? I wouldn't recommend using such
a tool, but with this patch, it will zap all the tablespaces.
Before this, you at least got a working database and could
read out all the data or fix the symlinks afterwards.
Yeah, but I don't think user will do such a thing without
being aware of the same and if he is aware, he will either
restore the symlinks before starting the server or would
atleast keep a copy of such a backup until he is able to
restore the database completely.
Do you think adding a note in docs makes sense?
How about if we simply abort if we find a non-symlink where we want the
symlink to be, and only remove something that is actually a symlink (or
a junction point, which is more or less the same thing)? Then it would
be up to the user to recover the situation, by moving or removing the
offending file/directory, and trying again.
cheers
andrew
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: Abbreviated keys for Datum tuplesort (was: Re: B-Tree support function number 3 (strxfrm() optimization))
On Wed, May 13, 2015 at 2:44 PM, Peter Geoghegan p...@heroku.com wrote: On Wed, May 13, 2015 at 11:40 AM, Robert Haas robertmh...@gmail.com wrote: Committed. Thanks for the patch and your patience. This comment was not updated: /* * The sortKeys variable is used by every case other than the datum and * hash index cases; it is set by tuplesort_begin_xxx. tupDesc is only * used by the MinimalTuple and CLUSTER routines, though. */ TupleDesc tupDesc; SortSupport sortKeys; /* array of length nKeys */ Oops. Fixed. Thanks. -- 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] Proposal : REINDEX xxx VERBOSE
On Wed, May 13, 2015 at 2:10 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Uh, are we really using INFO to log this? I thought that was a discouraged level to use anymore. Why not NOTICE? Well, as Masahiko-san points out, VACUUM uses INFO. I can't see any good reason to make this different. Also, when multiple tables are reindexed, do we emit lines for each index, or only for each table? If we're going to emit a line for each index in the single-table mode, it seems more sensible to do the same for the multi-table forms also. Hmm, yeah, I agree with that. I thought the patch worked that way, but I see now that it doesn't. I think that should be changed. -- 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] PATCH: adaptive ndistinct estimator v4
On Thu, Apr 30, 2015 at 6:20 PM, Tomas Vondra tomas.von...@2ndquadrant.com
wrote:
Hi,
On 04/30/15 22:57, Robert Haas wrote:
On Tue, Mar 31, 2015 at 3:02 PM, Tomas Vondra
tomas.von...@2ndquadrant.com wrote:
attached is v4 of the patch implementing adaptive ndistinct estimator.
So, I took a look at this today. It's interesting work, but it looks
more like a research project than something we can commit to 9.5. As
far as I can see, this still computes the estimate the way we do
today, but then also computes the estimate using this new method.
The estimate computed the new way isn't stored anywhere, so this
doesn't really change behavior, except for printing out a WARNING
comparing the values produced by the two estimators.
I agree that this is not ready for 9.5 - it was meant as an experiment
(hence printing the estimate in a WARNING, to make it easier to compare
the value to the current estimator). Without that it'd be much more
complicated to compare the old/new estimates, but you're right this is
not suitable for commit.
With the warning it is very hard to correlate the discrepancy you do see
with which column is causing it, as the warnings don't include table or
column names (Assuming of course that you run it on a substantial
database--if you just run it on a few toy cases then the warning works
well).
If we want to have an explicitly experimental patch which we want people
with interesting real-world databases to report back on, what kind of patch
would it have to be to encourage that to happen? Or are we never going to
get such feedback no matter how friendly we make it? Another problem is
that you really need to have the gold standard to compare them to, and
getting that is expensive (which is why we resort to sampling in the first
place). I don't think there is much to be done on that front other than
bite the bullet and just do it--perhaps only for the tables which have
discrepancies.
Some of the regressions I've seen are at least partly a bug:
+ /* find the 'm' value minimizing the difference */
+ for (m = 1; m = total_rows; m += step)
+ {
+ double q = k / (sample_rows * m);
sample_rows and m are both integers, and their product overflows
vigorously. A simple cast to double before the multiplication fixes the
first example I produced. The estimate goes from 137,177 to 1,108,076.
The reality is 1,062,223.
Perhaps m should be just be declared a double, as it is frequently used in
double arithmetic.
Leaving that aside, at some point, you'll say, OK, there may be some
regressions left but overall I believe this is going to be a win in
most cases. It's going to be really hard for anyone, no matter how
smart, to figure out through code review whether that is true. So
committing this figures to be extremely frightening. It's just not
going to be reasonably possible to know what percentage of users are
going to be more happy after this change and what percentage are
going to be less happy.
For every pair of estimators you can find cases where one of them is
better than the other one. It's pretty much impossible to find an estimator
that beats all other estimators on all possible inputs.
There's no way to make this an improvement for everyone - it will produce
worse estimates in some cases, and we have to admit that. If we think this
is unacceptable, we're effectively stuck with the current estimator forever.
Therefore, I think that:
1. This should be committed near the beginning of a release cycle,
not near the end. That way, if there are problem cases, we'll have a
year or so of developer test to shake them out.
It can't hurt, but how effective will it be? Will developers know or care
whether ndistinct happened to get better or worse while they are working on
other things? I would think that problems will be found by focused
testing, or during beta, and probably not by accidental discovery during
the development cycle. It can't hurt, but I don't know how much it will
help.
2. There should be a compatibility GUC to restore the old behavior.
The new behavior should be the default, because if we're not
confident that the new behavior will be better for most people, we
have no business installing it in the first place (plus few people
will try it). But just in case it turns out to suck for some people,
we should provide an escape hatch, at least for a few releases.
I think a compatibility GUC is a damn poor solution, IMNSHO.
For example, GUCs are database-wide, but I do expect the estimator to
perform worse only on a few data sets / columns. So making this a
column-level settings would be more appropriate, I think.
But it might work during the development cycle, as it would make comparing
the estimators possible (just run the tests with the GUC set differently).
Assuming we'll re-evaluate it at the end, and remove the GUC if possible.
I agree with the experimental GUC. That way if hackers do happen to
[HACKERS] i feel like compelled !
hi i don't want to make this post long. i just started to use PostgreSQL and it is absolutely awesome . just want to say thank you very much
Re: [HACKERS] Final Patch for GROUPING SETS
On 2015-05-12 05:36:19 +0200, Andres Freund wrote: Another controversial item was the introduction of GroupedVar. The need for this can be avoided by explicitly setting to NULL the relevant columns of the representative group tuple when evaluating result rows, but (a) I don't think that's an especially clean approach (though I'm not pushing back very hard on it) and (b) the logic needed in its absence is different between the current chaining implementation and a possible union implementation, so I decided against making any changes on wasted-effort grounds. Seems like fairly minor point to me. I very tentatively lean towards setting the columns in the group tuple to NULL. I'm pretty sure by now that I dislike the introduction of GroupedVar, and not just tentatively. While I can see why you found its introduction to be nicer than fiddling with the result tuple, for me the disadvantages seem to outweigh the advantage. For one it's rather wierd to have Var nodes be changed into GroupedVar in setrefs.c. The number of places that need to be touched even when it's a 'planned stmt only' type of node is still pretty large. Andrew: I'll work on changing this in a couple hours unless you're speaking up about doing it yourself. Greetings, Andres Freund -- 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] Proposal : REINDEX xxx VERBOSE
On Thu, May 14, 2015 at 3:10 AM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Uh, are we really using INFO to log this? I thought that was a discouraged level to use anymore. Why not NOTICE? I think it should be INFO level because it is a information of REINDEX command,such as progress of itself, CPU usage and so on. it would be overkill if we output the logs as NOTICE level, and verbose outputs of other maintenance command are emitted as INFO level. Also, when multiple tables are reindexed, do we emit lines for each index, or only for each table? If we're going to emit a line for each index in the single-table mode, it seems more sensible to do the same for the multi-table forms also. I agree that we emit lines for each table when we do reindex multiple tables. The latest patch is attached. Regards, --- Sawada Masahiko 000_reindex_verbose_v15.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] KNN-GiST with recheck
On 04/17/2015 12:05 PM, Alexander Korotkov wrote: On Wed, Feb 25, 2015 at 12:15 PM, Alexander Korotkov aekorot...@gmail.com wrote: Hi! On Tue, Feb 24, 2015 at 5:39 PM, Tomas Vondra tomas.von...@2ndquadrant.com wrote: On 17.2.2015 14:21, Alexander Korotkov wrote: On Sun, Feb 15, 2015 at 2:08 PM, Alexander Korotkov aekorot...@gmail.com mailto:aekorot...@gmail.com wrote: Revised patch with reordering in GiST is attached (knn-gist-recheck-in-gist.patch) as well as testing script (test.py). I meant to do a bit of testing on this (assuming it's still needed), but the patches need rebasing - Heikki fixed a few issues, so they don't apply cleanly. Both patches are revised. Both patches are rebased against current master. This looks pretty much ready. I'm going to spend some time on this on Friday, and if all looks good, commit. (Thursday's a public holiday here). One quick comment: It would be good to avoid the extra comparisons of the distances, when the index doesn't return any lossy items. As the patch stands, it adds one extra copyDistances() call and a cmp_distances() call for each tuple (in a knn-search), even if there are no lossy tuples. - Heikki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] pgAdmin4 Bug fix or my Fault ?
Hi, I am using Debian Jessie and install pgAdmin4 Required modules. after I clone pgAdmin4 from http://git.postgresql.org/gitweb/?p=pgadmin4.git;a=summary , First of, ıf I want run setup.py, I must fix bug . after I want run pgadmin4.py, I must fix gravatar import line. In this case, There are really bugs? or is it my fault ? Thank you. diff --git a/web/pgadmin/browser/views.py b/web/pgadmin/browser/views.py index 07637be..f336f31 100644 --- a/web/pgadmin/browser/views.py +++ b/web/pgadmin/browser/views.py @@ -11,7 +11,7 @@ MODULE_NAME = 'browser' from flask import Blueprint, Response, current_app, render_template, url_for -from flaskext.gravatar import Gravatar +from flask_gravatar import Gravatar from flask.ext.security import login_required from flask.ext.login import current_user from inspect import getmoduleinfo, getmembers @@ -223,4 +223,4 @@ def get_nodes(): return resp - \ No newline at end of file + diff --git a/web/setup.py b/web/setup.py index c7398f5..da6652e 100644 --- a/web/setup.py +++ b/web/setup.py @@ -59,7 +59,9 @@ def do_setup(): db.create_all() user_datastore.create_role(name='Administrators', description='pgAdmin Administrators Role') user_datastore.create_user(email=email, password=password) +db.session.commit() user_datastore.add_role_to_user(email, 'Administrators') + # Get the user's ID and create the default server group user = User.query.filter_by(email=email).first() @@ -138,4 +140,5 @@ if os.path.isfile(config.SQLITE_PATH): do_upgrade() else: print The configuration database %s does not exist.\nEntering initial setup mode...\n % config.SQLITE_PATH -do_setup() \ No newline at end of file +do_setup() -- Seçkin ALAN http://sckn.org -- 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] i feel like compelled !
On 14/05/15 09:35, essam Gndelee essam wrote: hi i don't want to make this post long. i just started to use PostgreSQL and it is absolutely awesome . just want to say thank you very much You're allowed to say more!!! :-) What particular features do you like best, and why? What O/S, hardware, use cases (what are you using pg for), volumes of data, performance... What other Databases have you used? How much experience do you have? Cheers, Gavin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix token exceeding NAMELEN
On 13 May 2015 at 17:55, Tom Lane t...@sss.pgh.pa.us wrote: Aaron W. Swenson titanof...@gentoo.org writes: Trying to build HEAD and ran into this issue building the docs: openjade:logicaldecoding.sgml:575:62:Q: length of name token must not exceed NAMELEN (44) openjade:replication-origins.sgml:87:67:Q: length of name token must not exceed NAMELEN (44) Hmm ... that's odd. I don't see any such failure here, and the buildfarm members that build the docs aren't complaining either. What version of openjade are you using exactly? So, I've attached a patch that'll fix it. I have no particular objection to the patch as stated, but I'm just wondering if this is the tip of a tool compatibility iceberg we were not previously aware of. I recall us hitting this with Slony documentation. The NAMELEN limit lay in the SGML/DocBook configuration that was configured at the distribution level, so that it differed (crucially) betwen Debian and Red Hat. Red Hat used to have a lower name length limit, and while overriding it was technically possible, it required modifying configuration that the distribution thought was owned by one of the SGML packages, and hence the modification seemed pretty inadvisable. I thought that this restriction was alleviated years ago, so I'm a bit surprised to see this come up in 2015. (Or perhaps Gentoo hasn't yet opened up some limits??? :-) ) -- When confronted by a difficult problem, solve it by reducing it to the question, How would the Lone Ranger handle this?
Re: [HACKERS] Fix token exceeding NAMELEN
Aaron W. Swenson titanof...@gentoo.org writes: Trying to build HEAD and ran into this issue building the docs: openjade:logicaldecoding.sgml:575:62:Q: length of name token must not exceed NAMELEN (44) openjade:replication-origins.sgml:87:67:Q: length of name token must not exceed NAMELEN (44) Hmm ... that's odd. I don't see any such failure here, and the buildfarm members that build the docs aren't complaining either. What version of openjade are you using exactly? So, I've attached a patch that'll fix it. I have no particular objection to the patch as stated, but I'm just wondering if this is the tip of a tool compatibility iceberg we were not previously aware of. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PATCH: pgbench allow '=' in \set
On Wed, May 13, 2015 at 3:54 PM, Fabien COELHO coe...@cri.ensmp.fr wrote: Ok. I've marked this one as REJECTED. Otherwise, I was considering this kind of things: n := expr n, m, p, q := SELECT ... Also, having ; as a end of commands could also help by allowing multiline commands, but that would break compatibility. Maybe allowing continuations (\\\n) would be an acceptable compromise. It's been my assumption that we wanted to keep the existing pgbench syntax mostly intact, and extend it. We could, of course, invent a completely new syntax, but it might be hard to get everybody to agree on what the new thing should look like. I loathe violently the convention of using a backslash at the end of a line, because it's too easy to write backslash-space-newline or backslash-tab-newline when you meant to write backslash-newline. But maybe we should do it anyway. We certainly need some solution to that problem, because the status quo is monumentally annoying, and that might be the least bad solution available. Another option, breaking backward compatibility, would be to decide that backslash commands have to be terminated by a semicolon token. Then we wouldn't need a continuation character, because we'd just continue across lines until we hit the terminator. Of course, that doesn't solve the problem for people who want to include multi-line SQL queries. If we wanted to make that work, the best option might be to duplicate the backend lexer into pgbench just as we already do with psql. Then it could identify properly-terminated SQL queries automatically. That, too, would be a backward compatibility break, since the terminating semicolon would become required there as well. I somewhat lean toward this second option, because I think it will be a lot more convenient in the long run. We'll probably get some complains about breaking people's pgbench scripts, but I'd personally be prepared to accept that as the price of progress. -- 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] BRIN range operator class
Emre Hasegeli wrote:
I pushed patches 04 and 07, as well as adopting some of the changes to
the regression test in 06. I'm afraid I caused a bit of merge pain for
you -- sorry about that.
No problem. I rebased the remaining ones.
Thanks!
After some back-and-forth between Emre and me, here's an updated patch.
My changes are cosmetic; for a detailed rundown, see
https://github.com/alvherre/postgres/commits/brin-inclusion
Note that datatype point was removed: it turns out that unless we get
box_contain_pt changed to use FPlt() et al, indexes created with this
opclass would be corrupt. And we cannot simply change box_contain_pt,
because that would break existing GiST and SP-GiST indexes that use it
today and pg_upgrade to 9.5! So that needs to be considered separately.
Also, removing point support means remove the CAST support procedure,
because there is no use for it in the supported types. Also, patch 05
in the previous submissions goes away completely because there's no need
for those (box,point) operators anymore.
There's nothing Earth-shattering here that hasn't been seen in previous
submissions by Emre.
One item of note is that this patch is blindly removing the assert-only
blocks as previously discussed, without any replacement. Need to think
more on how to put something back ...
--
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training Services
diff --git a/doc/src/sgml/brin.sgml b/doc/src/sgml/brin.sgml
index 92dac7c..b26bcf3 100644
--- a/doc/src/sgml/brin.sgml
+++ b/doc/src/sgml/brin.sgml
@@ -72,7 +72,9 @@
para
The firsttermminmax/
operator classes store the minimum and the maximum values appearing
- in the indexed column within the range.
+ in the indexed column within the range. The firstterminclusion/
+ operator classes store a value which includes the values in the indexed
+ column within the range.
/para
table id=brin-builtin-opclasses-table
@@ -252,6 +254,18 @@
/entry
/row
row
+ entryliteralinet_inclusion_ops/literal/entry
+ entrytypeinet/type/entry
+ entry
+ literalamp;amp;/
+ literalgt;gt;/
+ literalgt;gt;=/
+ literallt;lt;/literal
+ literallt;lt;=/literal
+ literal=/literal
+ /entry
+/row
+row
entryliteralbpchar_minmax_ops/literal/entry
entrytypecharacter/type/entry
entry
@@ -373,6 +387,25 @@
/entry
/row
row
+ entryliteralrange_inclusion_ops//entry
+ entrytypeany range type/type/entry
+ entry
+ literalamp;amp;/
+ literalamp;gt;/
+ literalamp;lt;/
+ literalgt;gt;/
+ literallt;lt;/
+ literallt;@/
+ literal=/
+ literal@gt;/
+ literallt;/literal
+ literallt;=/literal
+ literal=/literal
+ literalgt;=/literal
+ literalgt;/literal
+ /entry
+/row
+row
entryliteralpg_lsn_minmax_ops/literal/entry
entrytypepg_lsn/type/entry
entry
@@ -383,6 +416,43 @@
literalgt;/literal
/entry
/row
+row
+ entryliteralbox_inclusion_ops//entry
+ entrytypebox/type/entry
+ entry
+ literalamp;amp;/
+ literalamp;gt;/
+ literalamp;lt;/
+ literalgt;gt;/
+ literallt;lt;/
+ literallt;@/
+ literal~=/
+ literal@gt;/
+ literalamp;gt;|/
+ literal|amp;lt;/
+ literalgt;gt;|/
+ literal|lt;lt;/literal
+ /entry
+/row
+row
+ entryliteralpoint_box_inclusion_ops//entry
+ entrytypepoint/type/entry
+ entry
+ literalamp;amp;/
+ literalamp;gt;/
+ literalamp;lt;/
+ literalgt;gt;/
+ literallt;lt;/
+ literallt;@/
+ literal~=/
+ literalamp;gt;|/
+ literal|amp;lt;/
+ literalgt;gt;|/
+ literal|lt;lt;/literal
+ literalgt;^/
+ literal|lt;^/literal
+ /entry
+/row
/tbody
/tgroup
/table
diff --git a/src/backend/access/brin/Makefile b/src/backend/access/brin/Makefile
index ac44fcd..fb36882 100644
--- a/src/backend/access/brin/Makefile
+++ b/src/backend/access/brin/Makefile
@@ -13,6 +13,6 @@ top_builddir = ../../../..
include $(top_builddir)/src/Makefile.global
OBJS = brin.o brin_pageops.o brin_revmap.o brin_tuple.o brin_xlog.o \
- brin_minmax.o
+ brin_minmax.o brin_inclusion.o
include $(top_srcdir)/src/backend/common.mk
diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index 2b5fb8d..1995125 100644
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -105,11 +105,6 @@ brininsert(PG_FUNCTION_ARGS)
BrinMemTuple *dtup;
BlockNumber heapBlk;
int keyno;
-#ifdef USE_ASSERT_CHECKING
- BrinTuple *tmptup;
- BrinMemTuple *tmpdtup;
- Size tmpsiz;
-#endif
CHECK_FOR_INTERRUPTS();
@@ -137,45 +132,6 @@ brininsert(PG_FUNCTION_ARGS)
dtup = brin_deform_tuple(bdesc, brtup);
-#ifdef USE_ASSERT_CHECKING
- {
- /*
- * When assertions are enabled, we use this as an opportunity
Re: [HACKERS] i feel like compelled !
Oh well... then, THANKS GUYS!!! I'm not the original poster, btw. I felt a bit 'abandoned' a while back, since I started using Interbase/Firebird since, like, ~2000. But since Firebird never really took off, I felt I had to look for better solutions. I worked with Oracle for a bit, and then MySQL. But I found Oracle to be expensive (obviously) and too intrusive into my OS (Linux, many flavours, but mostly RedHat-based). What I really liked about Firebird, and then Postgres made me feel right at home, was standards-compliance with SQL and great feature set. I find myself most-often-than-not guessing how something ought to work in Postgres, based on past experiences, and finding that it works exactly (mostly) like it 'should'. Plus, I found many new things that I loved and changed the way I think about stuff, like using Python for SP, JSON fields and RegEx in WHERE. And a special mention to the Async NOTIFY stuff which finally works like it 'should' in a DB (Firebird had something like that, but with no payload). Also, how postgres is easy to deploy really helps. For example, I use it with a Qt App, which is compiled in MinGW. So I recompiled libpq with the same compiler, thus avoiding extra DLLs. Thanks! On Thursday 14 May 2015 09:48:35 Gavin Flower wrote: On 14/05/15 09:35, essam Gndelee essam wrote: hi i don't want to make this post long. i just started to use PostgreSQL and it is absolutely awesome . just want to say thank you very much You're allowed to say more!!! :-) What particular features do you like best, and why? What O/S, hardware, use cases (what are you using pg for), volumes of data, performance... What other Databases have you used? How much experience do you have? Cheers, Gavin
Re: [HACKERS] pgAdmin4 Bug fix or my Fault ?
I think you want PGAdmin support: http://www.pgadmin.org/support/list.php Also, why isn't the non-subscribe email address listed on that webpage? --- On Thu, May 14, 2015 at 12:11:15AM +0300, Seçkin Alan wrote: Hi, I am using Debian Jessie and install pgAdmin4 Required modules. after I clone pgAdmin4 from http://git.postgresql.org/gitweb/?p=pgadmin4.git;a=summary , First of, ıf I want run setup.py, I must fix bug . after I want run pgadmin4.py, I must fix gravatar import line. -- 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] Proposal : REINDEX xxx VERBOSE
On Thu, May 14, 2015 at 4:53 AM, Robert Haas robertmh...@gmail.com wrote: On Wed, May 13, 2015 at 2:10 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Uh, are we really using INFO to log this? I thought that was a discouraged level to use anymore. Why not NOTICE? Well, as Masahiko-san points out, VACUUM uses INFO. I can't see any good reason to make this different. Also, when multiple tables are reindexed, do we emit lines for each index, or only for each table? If we're going to emit a line for each index in the single-table mode, it seems more sensible to do the same for the multi-table forms also. Hmm, yeah, I agree with that. I thought the patch worked that way, but I see now that it doesn't. I think that should be changed. The v15 patch emits a line for each table when reindexing multiple tables, and emits a line for each index when reindexing single table. But v14 patch emits a line for each index, regardless of reindex target. Should I change back to v14 patch? Regards, --- Sawada Masahiko -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposal : REINDEX xxx VERBOSE
On Wed, May 13, 2015 at 2:49 PM, Sawada Masahiko sawada.m...@gmail.com wrote: On Thu, May 14, 2015 at 12:28 AM, Robert Haas robertmh...@gmail.com wrote: On Thu, May 7, 2015 at 6:55 PM, Sawada Masahiko sawada.m...@gmail.com wrote: Sorry, I forgot attach files. Review comments: - Customarily we use int, rather than uint8, for flags variables. I think we should do that here also. - reindex_index() emits a log message either way, but at DEBUG2 level without VERBOSE and at the INFO level with it. I think we should skip it altogether without VERBOSE. i.e. if (options REINDEXOPT_VERBOSE) ereport(...) - The errmsg() in that function should not end with a period. - The 000 patch makes a pointless whitespace change to tab-complete.c. - Instead of an enumerated type (ReindexOption) just use #define to define the flag value. Apart from those fairly minor issues, I think this looks pretty solid. Thank you for reviwing.. All fixed. IMHO we don't need pg_rusage_init(ru0) if the verbose options is not setted. Maybe change: + +pg_rusage_init(ru0); to + +if (options REINDEXOPT_VERBOSE) +pg_rusage_init(ru0); Regards, -- Fabrízio de Royes Mello Consultoria/Coaching PostgreSQL Timbira: http://www.timbira.com.br Blog: http://fabriziomello.github.io Linkedin: http://br.linkedin.com/in/fabriziomello Twitter: http://twitter.com/fabriziomello Github: http://github.com/fabriziomello
[HACKERS] Triaging the remaining open commitfest items
Looking at what remains open in the current commitfest: * fsync $PGDATA recursively at startup Andres is the reviewer of record on this one. He should either commit it if he feels it's ready, or bounce it to next CF if not. * EvalPlanQual behaves oddly for FDW queries involving system columns I've been working this one and will deal with any mop-up needed, but I think probably what ought to happen now is just to commit the small ctid-transmission hack I posted yesterday and call it good. * BRIN inclusion operator class This is Alvaro's turf, obviously. * PageRepairFragmentation optimization Heikki's patch, so his to commit or not, though since it's marked Waiting on Author I'm guessing it's not ready? * Abbreviated key support for Datum sorts I've been assuming Robert would either commit this or not, since he's been the committer for the predecessor patches. * KNN-GiST with recheck Heikki's been dealing with this thread so far, and is surely the best qualified to decide if it's ready or not. * GIN fillfactor I'd like to put this one on Heikki's plate as well, since he's touched the GIN code more than anyone else lately. * Manipulating complex types as non-contiguous structures in-memory This one's mine of course. I've been hoping to get more independent performance testing than it's gotten, but time grows short. I'm inclined to just go ahead and push it in. * Optimization for updating foreign tables in Postgres FDW I concur with Stephen's assessment that this doesn't look well designed. I think we should just mark it RWF for now. * iteration over record in plpgsql I fear this one slipped through the cracks --- it's marked Waiting on Author in the CF app, but it looks like Pavel did submit an updated version after that marking was made. However, it's not a terribly significant feature and there was doubt as to whether we want it at all anyway. Suggest we just punt it to next CF at this point. * Sequence Access Method Heikki's marked as reviewer, so it's his call as to whether this is ready, but the impression I have is that there's not really consensus as to whether the API is good. If not, it's something I think we should push to 9.6. * archive_mode=shared/always Heikki's patch, so his call. * Sending WAL receiver feedback regularly even if the receiver is under heavy load This seems to not be ready, but it also seems to be a bug fix, so when it is ready I think we could commit it. * Auditing extension I do not get the impression that there is consensus on this. Push to 9.6? * ctidscan as an example of custom-scan This basically hasn't gotten any attention, which may mean nobody cares enough to justify putting it in the tree. We need to either push it to next CF or reject altogether. * parallel mode/contexts Robert's patch, his to deal with (likewise for assessing parallel-safety). * RLS: row-level security, more review Stephen's baby. * Cube extension kNN support This is still marked as needs review, I'm afraid we have to push to 9.6. * compress method for spgist * Join pushdown support for foreign tables There doesn't seem to be any current patch linked to this CF item. If there is a patch to get postgres_fdw to make use of the recently committed join-path support, I assume it's in need of a rebase anyway. * Grouping Sets I had originally promised to be committer for this one, and still want to go look at it, but Robert's nearby message about not committing stuff in haste definitely seems to apply. * TABLESAMPLE clause I assume we'd better push this to 9.6 at this point. * REINDEX xxx VERBOSE Seems like we just need somebody to make a decision on syntax. * Additional role attributes Is this ready to commit? Stephen's call. * catalog view to pg_hba.conf file Greg Stark is marked as committer of record on this. * Add pg_settings.pending_restart column Peter's patch, his call. So there you have it. If everyone would go make decisions on the patches that they are the most obvious committer for, we could get those taken care of one way or the other pretty quickly. As for the ones I proposed pushing to 9.6, any committer who feels differently can pick those up, else I'll go change their status in the CF app tomorrow or so. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] a few thoughts on the schedule
On Wed, May 13, 2015 at 11:19:29AM -0400, Bruce Momjian wrote: On Wed, May 13, 2015 at 10:32:03AM -0400, Robert Haas wrote: We also need to start thinking about what happens after feature freeze. The CommitFest application currently lists a 2015-06 CommitFest which, according to previous practice, would be expected to start on the 15th of the month. Given where we are now, that seems entirely unrealistic. I am doubtful that we will be ready to ship a beta by mid-June, let alone begin a new development cycle. This is a very good analysis. I have been holding back my trivial new patches for 9.6 in hopes of setting a good example. However, all my stuff is new, while many of these other patches have waited their turn for review, and we are going to be unfair to submitters if we don't give them the attention they deserve --- that is always the tension we have at this time. We have three days left so I think we need committers to devote serious time, if possible, to helping us resolve as much as we can. If we start thinking about this on Friday, it is too late. Let me put a finer point on this --- whatever gets pushed to 9.6 unreasonably will be a feature we don't have in 9.5 and will discourage future development. I know we can't do magic, but now is the time to try. -- 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] Triaging the remaining open commitfest items
Tom, all, * Tom Lane (t...@sss.pgh.pa.us) wrote: * Optimization for updating foreign tables in Postgres FDW I concur with Stephen's assessment that this doesn't look well designed. I think we should just mark it RWF for now. I had meant to do that already, sorry about that, done now. * iteration over record in plpgsql I fear this one slipped through the cracks --- it's marked Waiting on Author in the CF app, but it looks like Pavel did submit an updated version after that marking was made. However, it's not a terribly significant feature and there was doubt as to whether we want it at all anyway. Suggest we just punt it to next CF at this point. I had been interested but also thought it was waiting for a new version. Doesn't look like I'll have time now. * Auditing extension I do not get the impression that there is consensus on this. Push to 9.6? I've not seen much disagreement about what it provides recently, those were dealt with a while ago. I agree with Robert that it needs more work on documentation and comments and I've sent my thoughts about what to improve in those areas over to David. I've reviewed it in various forms and am hoping to commit it, unless there are objections. * RLS: row-level security, more review Stephen's baby. I don't have anything pending on this at the moment. Post feature-freeze I'm planning to spend time on bug hunting, documentation improvements, etc, for this, UPSERT, and other things. If anyone is aware of any outstanding issues here, please let me know. * Additional role attributes Is this ready to commit? Stephen's call. I'm pretty happy with the latest version. Would be great for others to weigh in on if they have any concerns about reserving the 'pg_' prefix for system roles (or if they're fine with it, that'd be useful to know too..). I'll also be improving the documentation for it. * catalog view to pg_hba.conf file Greg Stark is marked as committer of record on this. I was hoping to look at that also, as I do think it'd be valuable to have. The current patch needs rework though. I agree with Peter that using keyword_* arrays is not a good approach and that it'd really be better to have this in conjunction with a function that users could use to see what row is returned. I might have time to work on it tomorrow, if other things fall into place, but it's not going to be without changes and perhaps that means it has to punt to 9.6. So there you have it. If everyone would go make decisions on the patches that they are the most obvious committer for, we could get those taken care of one way or the other pretty quickly. As for the ones I proposed pushing to 9.6, any committer who feels differently can pick those up, else I'll go change their status in the CF app tomorrow or so. Done, for my part. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] subxcnt defined as signed integer in SnapshotData and SerializeSnapshotData
On Thu, May 14, 2015 at 1:03 AM, Michael Paquier wrote: On Thu, May 14, 2015 at 12:02 AM, Robert Haas wrote: I'm just going to remove the useless assertion for now. What you're proposing here may (or may not) be worth doing, but it carries a non-zero risk of breaking something somewhere, if anyone is relying on the signed-ness of that type. Removing the assertion is definitely safe. Fine for me. That's indeed possible for an extension. Btw, I think that your commit message should have given some credit to Coverity for finding the problem. Not a big deal though. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Triaging the remaining open commitfest items
On 05/13/2015 11:38 AM, Tom Lane wrote: * Grouping Sets I had originally promised to be committer for this one, and still want to go look at it, but Robert's nearby message about not committing stuff in haste definitely seems to apply. That makes me sad. I wish you would still try. Sadly, my plate is full with other stuff, the last few days absorbed more than my available time. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Triaging the remaining open commitfest items
I wrote: * Cube extension kNN support This is still marked as needs review, I'm afraid we have to push to 9.6. * compress method for spgist Sigh. There was a Ditto under this one, but somehow it disappeared in editing :-( 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] Default Roles (was: Additional role attributes)
* Robert Haas (robertmh...@gmail.com) wrote: Yes: let's punt this to 9.6. The decisions you're making here are way too significant to be making a couple of days before feature freeze, and this patch has changed massively since it was first submitted. There isn't time now for people who want to have an opinion on this to form an educated one. Perhaps I'm missing something, but the patch has been simplified down to the point where the only question seems to be should we have default roles or not?, which I had asked about two weeks ago and again last week on a new thread. I feel like we're waiting for the silent majority to chime in. Put another way, I'm afraid that posting this next week, next month, or next year is going to garner just as many responses as it's seen in the past 2 weeks, while I continue to field questions on -bugs, -admin, and IRC about how do I set up Nagios with a non-superuser account? and similar issues. It's not a novel idea, certainly; Magnus suggested it back in December on the thread, Tom made a similar comment that it might make sense to have them later on and it's come up quite a few times previously as it's something other RDBMS's have and we don't. Clearly, others have read the proposal, at least (You and Alvaro on the other thread, Heikki on this one). It's my fault that I didn't follow up on their suggestions earlier and instead spent a bunch of time fighting with pg_dump, but it doesn't seem like there is a lot of disagreement about the idea. I'd offer to simplify it down, but it seems like the obvious change in that direction would be to reserve pg_ as a role prefix and not actually create any default roles, but that doesn't gain us anything and makes a potential headache for users without any feature to go with it. Bruce's point is a better one, except that all of the changes have been about reducing changes to core, down to an almost trivial level. I wish it had been a smoother ride to get here from the original proposal six months ago, but I've certainly got a better understanding of the level of effort involved and changes required for the other approaches and this certainly seems like the best and simplest. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] a few thoughts on the schedule
Bruce Momjian br...@momjian.us writes: Let me put a finer point on this --- whatever gets pushed to 9.6 unreasonably will be a feature we don't have in 9.5 and will discourage future development. I know we can't do magic, but now is the time to try. The other side of that coin is that the stuff that ends up getting pushed will, in many cases, be stuff that nobody cared a whole lot about. One thing that continues to bother me about the commitfest process is that it's created a default expectation that things get committed eventually. But many new ideas are just plain bad, and others are things that nobody but the author cares about. We need to remember that every new feature we add creates an ongoing maintenance burden, and might foreclose better ideas later. I'd like to see a higher threshold for accepting feature patches than we seem to have applied of late. 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] a few thoughts on the schedule
On 05/13/2015 08:09 AM, Tom Lane wrote: What I think we need to be doing this week is triage. Commit what's ready, punt what's not. I'll post a separate list about that. regards, tom lane +1 JD -- Command Prompt, Inc. - http://www.commandprompt.com/ 503-667-4564 PostgreSQL Centered full stack support, consulting and development. Announcing I'm offended is basically telling the world you can't control your own emotions, so everyone else should do it for you. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Default Roles (was: Additional role attributes)
On Wed, May 13, 2015 at 11:50 AM, Stephen Frost sfr...@snowman.net wrote: * Robert Haas (robertmh...@gmail.com) wrote: Yes: let's punt this to 9.6. The decisions you're making here are way too significant to be making a couple of days before feature freeze, and this patch has changed massively since it was first submitted. There isn't time now for people who want to have an opinion on this to form an educated one. Perhaps I'm missing something, but the patch has been simplified down to the point where the only question seems to be should we have default roles or not?, which I had asked about two weeks ago and again last week on a new thread. I feel like we're waiting for the silent majority to chime in. The thing is, right now, there are many, many patches in flight and everybody is really, really busy with them. What we should be trying to push in right now are the patches that we know we want, and there are at most a few minor implementation details to sort out. We should not be trying to push in any patches where we are not confident in the design. I really don't see how you can be confident that this design will have the backing of the community at this point. It's changed in major ways, multiple times. The latest version, again majorly revised, was posted TWO DAYS before feature freeze. Two days is not enough time to get meaningful feedback on significant design decisions under the best of circumstances, and even less so when those two days are the last remaining days before feature freeze. Now, if six people who are all well-known PostgreSQL contributors show up and they all say I looked at the latest version of this carefully and I'm confident you've got it right, then go ahead: push it. But don't make the mistake of thinking that because you're confident that you've now got it right everybody else will like it too. Even since you posted the last version, Heikki expressed a concern that resulted in (surprise!) more revisions. There comes a point where a patch that is still heavily in flux is just too late for the release cycle, and we're well past that point at this stage of the game. -- 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] Triaging the remaining open commitfest items
* Robert Haas (robertmh...@gmail.com) wrote: On Wed, May 13, 2015 at 11:38 AM, Tom Lane t...@sss.pgh.pa.us wrote: * Additional role attributes Is this ready to commit? Stephen's call. -1 for committing this, per discussion earlier today on a thread that's probably not linked into the CF app. Linked it into the CF, sorry for not doing so earlier. I'm not going to push it over objections, but I'm certainly hopeful that there will be further discussion on that thread. For my part, at least, it's a really nice capability to have that we've been missing for a long time- I can remember back to the first time I was setting up Nagios and wondering how in the world I could monitor the system without giving the Nagios user full superuser rights. I know there are instances out in the wild that I've set up which are still hacked up to deal with this lack of capability. Further, if we don't have something like this, then I'm worried about people who use the XLOG functions today for monitoring, as we'd end up having to lock those down to superuser-only, since there isn't anything between 'public' and 'superuser'. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] a few thoughts on the schedule
On Wed, May 13, 2015 at 11:52:40AM -0400, Tom Lane wrote: Bruce Momjian br...@momjian.us writes: Let me put a finer point on this --- whatever gets pushed to 9.6 unreasonably will be a feature we don't have in 9.5 and will discourage future development. I know we can't do magic, but now is the time to try. The other side of that coin is that the stuff that ends up getting pushed will, in many cases, be stuff that nobody cared a whole lot about. One thing that continues to bother me about the commitfest process is that it's created a default expectation that things get committed eventually. But many new ideas are just plain bad, and others are things that nobody but the author cares about. We need to remember that every new feature we add creates an ongoing maintenance burden, and might foreclose better ideas later. I'd like to see a higher threshold for accepting feature patches than we seem to have applied of late. Agreed. If the idea is good someone else will pick it up. -- 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] subxcnt defined as signed integer in SnapshotData and SerializeSnapshotData
On Thu, May 14, 2015 at 12:02 AM, Robert Haas robertmh...@gmail.com wrote: On Sat, May 9, 2015 at 8:48 AM, Michael Paquier michael.paqu...@gmail.com wrote: On Fri, May 8, 2015 at 10:27 PM, Simon Riggs wrote: On 8 May 2015 at 13:02, Michael Paquier wrote: I think that we should redefine subxcnt as uint32 for consistency with xcnt, and remove the two assertions that 924bcf4 has introduced. I could get a patch quickly done FWIW. (uint32) +1 Attached is the patch. This has finished by being far simpler than what I thought first. I'm just going to remove the useless assertion for now. What you're proposing here may (or may not) be worth doing, but it carries a non-zero risk of breaking something somewhere, if anyone is relying on the signed-ness of that type. Removing the assertion is definitely safe. Fine for me. That's indeed possible for an extension. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] a few thoughts on the schedule
On 2015-05-13 11:49:45 -0400, Robert Haas wrote: On Wed, May 13, 2015 at 11:40 AM, Bruce Momjian br...@momjian.us wrote: Let me put a finer point on this --- whatever gets pushed to 9.6 unreasonably will be a feature we don't have in 9.5 and will discourage future development. I know we can't do magic, but now is the time to try. And on the flip side, whatever is pushed to 9.6 will not create a stability issue in 9.5. I'm not sure it's that simple. For one I think patches don't age that well. Often enough patches don't really get significantly more review when they're delayed, and at the same time the understanding of the innards reduces again. But more importantly not integrating things that are either ready or nearly ready and that have been waiting for a long while, might be better for stability short term. But I think in the mid/long term it creates a lack of reviewers, contributors and eventually committers. Obviously that's *not* meant as a call to just commit everything pending. Greetings, Andres Freund -- 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] Triaging the remaining open commitfest items
Andres Freund and...@anarazel.de writes: On 2015-05-13 11:38:27 -0400, Tom Lane wrote: * Manipulating complex types as non-contiguous structures in-memory This one's mine of course. I've been hoping to get more independent performance testing than it's gotten, but time grows short. I'm inclined to just go ahead and push it in. I'm a bit hesitant about performance regressions around it. And I'd obviously rather not see the macros but the inline version ;). But I think overall we're in a better position with it, than without. If it turns out to have bad edge cases performancewise, we can still turn it off in plpgsql without much problems. If we, preferrably, can't find a better solution for the performance problem. Right, I should have said absorb Andres' input and then commit. What I wanted to know was whether there would be objections to committing this at all. 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] a few thoughts on the schedule
Andres Freund and...@anarazel.de writes: On 2015-05-13 11:52:40 -0400, Tom Lane wrote: One thing that continues to bother me about the commitfest process is that it's created a default expectation that things get committed eventually. Agreed that this is a problem. I think we need to work on giving that feedback rather sooner than later. It's one thing to be given a -1 a week or two after a patch gets proposed, another being given it 10 revisions and half a year later. How about we really try to triage the patches a) before the CF starts, b) half into the CF? We keep talking about doing something like that (I remember it's come up several times in the annual developer meetings), and then not actually doing it. But I agree it seems like a good idea. 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] a few thoughts on the schedule
On 05/13/2015 09:27 AM, Tom Lane wrote: Andres Freund and...@anarazel.de writes: On 2015-05-13 11:52:40 -0400, Tom Lane wrote: One thing that continues to bother me about the commitfest process is that it's created a default expectation that things get committed eventually. Agreed that this is a problem. I think we need to work on giving that feedback rather sooner than later. It's one thing to be given a -1 a week or two after a patch gets proposed, another being given it 10 revisions and half a year later. How about we really try to triage the patches a) before the CF starts, b) half into the CF? We keep talking about doing something like that (I remember it's come up several times in the annual developer meetings), and then not actually doing it. But I agree it seems like a good idea. What if: * Commitfest starts at branch. * Accept new patches for 9.6 until X date * At X date, tree is closed and patch review begins * Patch review continues until all patches are committed, kicked or Y date is met. * At Y date, we go to Alpha * At Z date, we go to Beta Well crap, I ran out of sequence letters but you get the idea. In short, no more ethereal We kind of do this on this date sort of. Stick to it, it may suck sometimes but it is really the only reasonable way to do it anymore. JD regards, tom lane -- Command Prompt, Inc. - http://www.commandprompt.com/ 503-667-4564 PostgreSQL Centered full stack support, consulting and development. Announcing I'm offended is basically telling the world you can't control your own emotions, so everyone else should do it for you. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposal : REINDEX xxx VERBOSE
On Thu, May 7, 2015 at 6:55 PM, Sawada Masahiko sawada.m...@gmail.com wrote: Sorry, I forgot attach files. Review comments: - Customarily we use int, rather than uint8, for flags variables. I think we should do that here also. - reindex_index() emits a log message either way, but at DEBUG2 level without VERBOSE and at the INFO level with it. I think we should skip it altogether without VERBOSE. i.e. if (options REINDEXOPT_VERBOSE) ereport(...) - The errmsg() in that function should not end with a period. - The 000 patch makes a pointless whitespace change to tab-complete.c. - Instead of an enumerated type (ReindexOption) just use #define to define the flag value. Apart from those fairly minor issues, I think this looks pretty solid. -- 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] a few thoughts on the schedule
* Bruce Momjian (br...@momjian.us) wrote: On Wed, May 13, 2015 at 11:19:29AM -0400, Bruce Momjian wrote: On Wed, May 13, 2015 at 10:32:03AM -0400, Robert Haas wrote: We also need to start thinking about what happens after feature freeze. The CommitFest application currently lists a 2015-06 CommitFest which, according to previous practice, would be expected to start on the 15th of the month. Given where we are now, that seems entirely unrealistic. I am doubtful that we will be ready to ship a beta by mid-June, let alone begin a new development cycle. This is a very good analysis. I have been holding back my trivial new patches for 9.6 in hopes of setting a good example. However, all my stuff is new, while many of these other patches have waited their turn for review, and we are going to be unfair to submitters if we don't give them the attention they deserve --- that is always the tension we have at this time. We have three days left so I think we need committers to devote serious time, if possible, to helping us resolve as much as we can. If we start thinking about this on Friday, it is too late. Let me put a finer point on this --- whatever gets pushed to 9.6 unreasonably will be a feature we don't have in 9.5 and will discourage future development. I know we can't do magic, but now is the time to try. I certainly agree with this and have been putting a fair bit of effort into it over the past week. :/ More help would absolutely be appreciated. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] Triaging the remaining open commitfest items
On Wed, May 13, 2015 at 11:38 AM, Tom Lane t...@sss.pgh.pa.us wrote: * fsync $PGDATA recursively at startup Andres is the reviewer of record on this one. He should either commit it if he feels it's ready, or bounce it to next CF if not. I committed the first part of this as 2ce439f3379aed857517c8ce207485655000fc8e. I think that we do not have design consensus on the rest. I think we should mark this committed, and if the folks who proposed the further work here still want to press their case, that should wait for 9.6. * Abbreviated key support for Datum sorts I've been assuming Robert would either commit this or not, since he's been the committer for the predecessor patches. I'll deal with this. * Sequence Access Method Heikki's marked as reviewer, so it's his call as to whether this is ready, but the impression I have is that there's not really consensus as to whether the API is good. If not, it's something I think we should push to 9.6. I share your concern on this one. * ctidscan as an example of custom-scan This basically hasn't gotten any attention, which may mean nobody cares enough to justify putting it in the tree. We need to either push it to next CF or reject altogether. Agreed. I was fine with never committing this. I don't think we have a requirement that every hook or bit of functionality we expose at the C level must have an example in core. But other people (you? Simon?) seemed to want a demonstration in the core repository. If that's still a priority, I am willing to work on it more for 9.6, but there is not time now. * parallel mode/contexts Robert's patch, his to deal with (likewise for assessing parallel-safety). Most of the parallel mode stuff is committed. What's left will have to wait for 9.6. Assessing parallel-safety will also need to wait for 9.6. * Join pushdown support for foreign tables There doesn't seem to be any current patch linked to this CF item. If there is a patch to get postgres_fdw to make use of the recently committed join-path support, I assume it's in need of a rebase anyway. I think there is a rebased patch around. I think it's just not linked into the CF thread. I don't think it's committable as is. * Grouping Sets I had originally promised to be committer for this one, and still want to go look at it, but Robert's nearby message about not committing stuff in haste definitely seems to apply. I really feel we didn't give this a fair shake. I'm not saying we should make up for that by committing it in haste, but not giving things a fair shake is bad for the project regardless of anything else. * TABLESAMPLE clause I assume we'd better push this to 9.6 at this point. +1 * REINDEX xxx VERBOSE Seems like we just need somebody to make a decision on syntax. I just posted a review of this raising minor points only. If it is timely revised, I will commit it. * Additional role attributes Is this ready to commit? Stephen's call. -1 for committing this, per discussion earlier today on a thread that's probably not linked into the CF app. * catalog view to pg_hba.conf file Greg Stark is marked as committer of record on this. I am doubtful whether this is ready. -- 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] a few thoughts on the schedule
On 2015-05-13 11:52:40 -0400, Tom Lane wrote: One thing that continues to bother me about the commitfest process is that it's created a default expectation that things get committed eventually. But many new ideas are just plain bad, and others are things that nobody but the author cares about. We need to remember that every new feature we add creates an ongoing maintenance burden, and might foreclose better ideas later. I'd like to see a higher threshold for accepting feature patches than we seem to have applied of late. Agreed that this is a problem. I think we need to work on giving that feedback rather sooner than later. It's one thing to be given a -1 a week or two after a patch gets proposed, another being given it 10 revisions and half a year later. How about we really try to triage the patches a) before the CF starts, b) half into the CF? I guess we'd have to somebody making a summary of each patch, and their own opinion. Then that list can be discussed. I don't really like that, because it involves a fair amount of work and has a good bit of potential for personal preferences to creep in. But I don't have a better idea. If necessary I'll do that for the first CF. Greetings, Andres Freund -- 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] Default Roles (was: Additional role attributes)
Robert Haas robertmh...@gmail.com writes: Now, if six people who are all well-known PostgreSQL contributors show up and they all say I looked at the latest version of this carefully and I'm confident you've got it right, then go ahead: push it. But don't make the mistake of thinking that because you're confident that you've now got it right everybody else will like it too. Even since you posted the last version, Heikki expressed a concern that resulted in (surprise!) more revisions. There comes a point where a patch that is still heavily in flux is just too late for the release cycle, and we're well past that point at this stage of the game. FWIW, I agree that we're past the point where we should be committing features whose external definition hasn't been stable for awhile. Fixing bugs post-feature-freeze is one thing, but if there's a significant chance that you'll be having to adjust the feature definition, then it's probably too late for 9.5. And this particular item sure looks like it's in that category. There's always another release cycle. 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] Triaging the remaining open commitfest items
On 13/05/15 17:38, Tom Lane wrote: * Sequence Access Method Heikki's marked as reviewer, so it's his call as to whether this is ready, but the impression I have is that there's not really consensus as to whether the API is good. If not, it's something I think we should push to 9.6. Heikki said he won't have time for this one before freeze so I guess it can be pushed to 9.6. -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Manipulating complex types as non-contiguous structures in-memory
On 2015-05-13 20:48:51 -0400, Tom Lane wrote: I still think that going back to defining the second byte as the size would be better. Fortunately, since this is only a matter of in-memory representations, we aren't committed to any particular answer. Requiring sizes to be different still strikes me as a disaster. Or is that not what you're proposing? -- 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] Proposal : REINDEX xxx VERBOSE
On Wed, May 13, 2015 at 8:25 PM, Sawada Masahiko sawada.m...@gmail.com wrote: The v15 patch emits a line for each table when reindexing multiple tables, and emits a line for each index when reindexing single table. But v14 patch emits a line for each index, regardless of reindex target. Should I change back to v14 patch? Uh, maybe. What made you change it? -- 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] Manipulating complex types as non-contiguous structures in-memory
Andres Freund and...@anarazel.de writes: On 2015-05-13 20:48:51 -0400, Tom Lane wrote: I still think that going back to defining the second byte as the size would be better. Fortunately, since this is only a matter of in-memory representations, we aren't committed to any particular answer. Requiring sizes to be different still strikes me as a disaster. Or is that not what you're proposing? It is, but why would it be a disaster? We could add StaticAsserts verifying that the sizes actually are different. I doubt that the pad space itself could amount to any issue performance-wise, since it would only ever exist in transient in-memory tuples, and even that only seldom. 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] Manipulating complex types as non-contiguous structures in-memory
Andres Freund and...@anarazel.de writes: On 2015-05-10 12:09:41 -0400, Tom Lane wrote: Andres Freund and...@anarazel.de writes: * The VARTAG_IS_EXPANDED(tag) trick in VARTAG_SIZE is unlikely to beneficial, before the compiler could implement the whole thing as a computed goto or lookup table, afterwards not. Well, if you're worried about the speed of VARTAG_SIZE() then the right thing to do would be to revert your change that made enum vartag_external distinct from the size of the struct, so that we could go back to just using the second byte of a varattrib_1b_e datum as its size. As I said at the time, inserting pad bytes to force each different type of toast pointer to be a different size would probably be a better tradeoff than what commit 3682025015 did. I doubt that'd be a net positive. Anyway, all I'm saying is that I can't see the VARTAG_IS_EXPANDED trick being beneficial in comparison to checking both explicit values. I did some microbenchmarking on this, and AFAICT doing it your way makes it slower. I still think that going back to defining the second byte as the size would be better. Fortunately, since this is only a matter of in-memory representations, we aren't committed to any particular answer. 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] Manipulating complex types as non-contiguous structures in-memory
Andres Freund and...@anarazel.de writes: On 2015-05-13 21:01:43 -0400, Tom Lane wrote: It is, but why would it be a disaster? We could add StaticAsserts verifying that the sizes actually are different. I doubt that the pad space itself could amount to any issue performance-wise, since it would only ever exist in transient in-memory tuples, and even that only seldom. The sizes would be platform dependant. So what? There are lots of platform-dependent constants in PG. It's also just incredibly ugly to have to add pad bytes to structures so we can disambiguate them. Well, I agree it's not too pretty, but you were the one who brought up the issue of the speed of VARTAG_SIZE(). We definitely gave up some performance there already, and my patch will make it worse. Anyway, I think we can live with your or my proposed additional branch for now. I can't see either variant being a relevant performance bottleneck anytime soon. Actually, after having microbenchmarked the difference between those two proposals, I'm not too sure that VARTAG_SIZE() is down in the noise. But it doesn't matter for the moment --- any one of these alternatives would be a very localized code change, and none of them would create an on-disk compatibility break. We can let it go until someone wants to put together a more definitive benchmark for testing. 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] Manipulating complex types as non-contiguous structures in-memory
Andres Freund and...@anarazel.de writes: On 2015-05-10 12:09:41 -0400, Tom Lane wrote: * I find the ARRAY_ITER_VARS/ARRAY_ITER_NEXT macros rather ugly. I don't buy the argument that turning them into functions will be slower. I'd bet the contrary on common platforms. Perhaps; do you want to do some testing and see? I've added new iterator functions using a on-stack state variable and array_iter_setup/next functions pretty analogous to the macros. And then converted arrayfuncs.c to use them. I confirm that this doesn't seem to be any slower (at least not on a compiler with inline functions). And it's certainly less ugly, so I've adopted it. Similarly using inline funcs for AARR_NDIMS/HASNULL does not appear to hamper performance and gets rid of the multiple evaluation risk. I'm less excited about that part though. The original ARR_FOO macros mostly have multiple-evaluation risks as well, and that's been totally academic so far. By the time you get done dealing with the STATIC_IF_INLINE dance, it's quite messy to have these be inline functions, and I am not seeing a useful return from adding the mess. 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] Triaging the remaining open commitfest items
* ctidscan as an example of custom-scan This basically hasn't gotten any attention, which may mean nobody cares enough to justify putting it in the tree. We need to either push it to next CF or reject altogether. Agreed. I was fine with never committing this. I don't think we have a requirement that every hook or bit of functionality we expose at the C level must have an example in core. But other people (you? Simon?) seemed to want a demonstration in the core repository. If that's still a priority, I am willing to work on it more for 9.6, but there is not time now. If no other people required it again, I don't think this module should be kept in core and also I'm not favor to push ctidscan to v9.6 development cycle. It intends to demonstrate custom-scan interface, however, it is not certain an example always needs to be in-core. If we split the patch partially, definition below makes sense to implement out-of-core example module +#define TIDNotEqualOperator402 DATA(insert OID = 2799 ( PGNSP PGUID b f f27 27 16 2800 DESCR(less than); #define TIDLessOperator2799 DATA(insert OID = 2800 ( PGNSP PGUID b f f27 27 16 2799 DESCR(greater than); +#define TIDGreaterOperator 2800 DATA(insert OID = 2801 ( =PGNSP PGUID b f f27 27 16 2802 DESCR(less than or equal); +#define TIDLessEqualOperator 2801 DATA(insert OID = 2802 ( =PGNSP PGUID b f f27 27 16 2801 DESCR(greater than or equal); +#define TIDGreaterEqualOperator2802 * Join pushdown support for foreign tables There doesn't seem to be any current patch linked to this CF item. If there is a patch to get postgres_fdw to make use of the recently committed join-path support, I assume it's in need of a rebase anyway. I think there is a rebased patch around. I think it's just not linked into the CF thread. I don't think it's committable as is. I believe he is working to follow up the latest foreign/custom-join interface on which postgres_fdw expected. One point we like to clarify is how create_plan_recurse() shall be dealt with. As Hanada-san posted yesterday, postgres_fdw internally uses create_plan_recurse() to fetch SQL statement associated with inner / outer sub-plan, so it will take additional adjustment work even though he already begin to work. | IMO it isn't necessary to apply the change onto | ForeignPath/ForeignScan. FDW would have a way to keep-and-use sub | paths as private information. In fact, I wrote postgres_fdw to use | create_plan_recurse to generate SQL statements of inner/outer | relations to be joined, but as of now SQL construction for join | push-down is accomplished by calling private function deparseSelectSql | recursively at the top of a join tree. Thanks, -- NEC Business Creation Division / PG-Strom Project KaiGai Kohei kai...@ak.jp.nec.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] upper planner path-ification
Hi, I've been pulling over Tom's occasional remarks about redoing grouping_planner - and maybe further layers of the planner - to work with Paths instead of Plans. I've had difficulty locating all of the relevant threads. Here's everything I've found so far, which I'm pretty sure is not everything: http://www.postgresql.org/message-id/17400.1311716...@sss.pgh.pa.us http://www.postgresql.org/message-id/2614.1375730...@sss.pgh.pa.us http://www.postgresql.org/message-id/22721.1385048...@sss.pgh.pa.us http://www.postgresql.org/message-id/banlktindjjfhnozesg2j2u4gokqlu69...@mail.gmail.com http://www.postgresql.org/message-id/8479.1418420...@sss.pgh.pa.us I think there are two separate problems here. First, there's the problem that grouping_planner() is complicated. It's doing cost comparisons, but all in ad-hoc fashion rather than using a consistent mechanic the way add_path() does. Generally, we can plan an aggregate using either (1) a hashed aggregate, (2) a sorted aggregate, or (3) for min or max, an index scan that just grabs the highest or lowest value in lieu of a full table scan. Instead of generating a plan for each of these possibilities, we'd like to generate paths for each one, and then pick one to turn into a plan. AIUI, the hope is that this would simplify the cost calculations, and also make it easier to inject other paths, such as a path where an FDW performs the aggregation step on the remote server. Second, there's the problem that we might like to order aggregates with respect to joins. If we have something like SELECT DISTINCT ON (foo.x) foo.x, foo.y, bar.y FROM foo, bar WHERE foo.x = bar.x, then (a) if foo.x has many duplicates, it will be better to DISTINCT-ify foo and then join to bar afterwards but (b) if foo.x = bar.x is highly selective, it will be better to join to bar first and then DISTINCT-ify the result. Currently, aggregation is always last; that's not great. Hitoshi Harada's proposed strategy of essentially figuring out where the aggregation steps can go and then re-planning for each one is also not great, because each join problem will be a subtree of the one we use for the aggregate-last strategy, and thus we're wasting effort by planning the same subtrees multiple times. Instead, we might imagine letting grouping planner fish out the best paths for the joinrels that represent possible aggregation points, generate aggregation paths for each of those, and then work out what additional rels need to be joined afterwards. That sounds hard, but not as hard as doing something sensible with what we have today. I'm inclined to think that it would be useful to solve the first problem even if we didn't solve the second one right away (but that might be wrong). As a preparatory step, I'm thinking it would be sensible to split grouping_planner() into an outer function that would handle the addition of Limit and LockRows nodes and maybe planning of set operations, and an inner function that would handle GROUP BY, DISTINCT, and possibly window function planning. Thoughts? -- 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] upper planner path-ification
On Wed, May 13, 2015 at 10:27 PM, Tom Lane t...@sss.pgh.pa.us wrote: Both of those are problems all right, but there is more context here. Thanks for providing the context. I'm inclined to think that it would be useful to solve the first problem even if we didn't solve the second one right away (but that might be wrong). As a preparatory step, I'm thinking it would be sensible to split grouping_planner() into an outer function that would handle the addition of Limit and LockRows nodes and maybe planning of set operations, and an inner function that would handle GROUP BY, DISTINCT, and possibly window function planning. For the reasons I mentioned, I'd like to get to a point where subquery_planner's output is Paths not Plans as soon as possible. But the idea of coarse representation of steps that we aren't trying to be smart about might be useful to save some labor in the short run. The zero-order version of that might be a single Path node type that represents do whatever grouping_planner would do, which we'd start to break down into multiple node types once we had the other APIs fixed. The problem I'm really interested in solving is gaining the ability to add additional aggregation strategies, such as letting an FDW do it remotely, or doing it in parallel. It seems to me that your proposed zero-order version of that wouldn't really get us terribly far toward that goal - it would be more oriented towards solving the other problems you mention, specifically adding more intelligence to setops and allowing parameterization of subqueries. Those things certainly have some value, but I think supporting alternate aggregation strategies is a lot more interesting. -- 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] Manipulating complex types as non-contiguous structures in-memory
On 2015-05-13 20:28:52 -0400, Tom Lane wrote: Andres Freund and...@anarazel.de writes: Similarly using inline funcs for AARR_NDIMS/HASNULL does not appear to hamper performance and gets rid of the multiple evaluation risk. I'm less excited about that part though. The original ARR_FOO macros mostly have multiple-evaluation risks as well, and that's been totally academic so far. Fair point. -- 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] Manipulating complex types as non-contiguous structures in-memory
On 2015-05-13 21:01:43 -0400, Tom Lane wrote: Andres Freund and...@anarazel.de writes: On 2015-05-13 20:48:51 -0400, Tom Lane wrote: I still think that going back to defining the second byte as the size would be better. Fortunately, since this is only a matter of in-memory representations, we aren't committed to any particular answer. Requiring sizes to be different still strikes me as a disaster. Or is that not what you're proposing? It is, but why would it be a disaster? We could add StaticAsserts verifying that the sizes actually are different. I doubt that the pad space itself could amount to any issue performance-wise, since it would only ever exist in transient in-memory tuples, and even that only seldom. The sizes would be platform dependant. It's also just incredibly ugly to have to add pad bytes to structures so we can disambiguate them. Anyway, I think we can live with your or my proposed additional branch for now. I can't see either variant being a relevant performance bottleneck anytime soon. -- 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] upper planner path-ification
Robert Haas robertmh...@gmail.com writes: I've been pulling over Tom's occasional remarks about redoing grouping_planner - and maybe further layers of the planner - to work with Paths instead of Plans. ... I think there are two separate problems here. First, there's the problem that grouping_planner() is complicated. Second, there's the problem that we might like to order aggregates with respect to joins. Both of those are problems all right, but there is more context here. * As some of the messages you cited mention, we would like to have Path representations for things like aggregation, because that's the only way we'll get to a sane API that lets FDWs propose remote aggregation. * We have also had requests for the planner to be smarter about UNION/INTERSECT/EXCEPT queries. Again, that requires cost comparisons, which would be better done if we had Path representations for the various ways we'd want to consider. Also, a big part of the issue there is wanting to be able to consider sorted versus unsorted plans for the leaf queries of the set-op (IOW, optionally pushing the sort requirements of the set-op down into the leaves). Right now, such comparisons are impossible because prepunion.c uses subquery_planner to handle the leaf queries, and what it gets back from that is one finished plan, not alternative Paths. * Likewise, subqueries-in-FROM are handled by recursing to subquery_planner, which gives us back just one frozen Plan for the subquery. Among other things this seems to make it too expensive to consider generating parameterized paths for the subquery. I'd like to keep subquery plans in Path form until much later as well. So these considerations motivate wishing that the result of subquery_planner could be a list of alternative Paths rather than a Plan, which means that every operation it knows how to tack onto the scan/join plan has to be representable by a Path of some sort. I don't know how granular that needs to be, though. For instance, one could certainly imagine that it might be sufficient initially to have a single WindowPath that represents do all the window functions, and then at create_plan time we'd generate multiple WindowAgg plan nodes in the same ad-hoc way as now. Breaking that down in the Path representation would only become interesting if it would affect higher-level decisions, and I'm not immediately seeing how it might do that. I'm inclined to think that it would be useful to solve the first problem even if we didn't solve the second one right away (but that might be wrong). As a preparatory step, I'm thinking it would be sensible to split grouping_planner() into an outer function that would handle the addition of Limit and LockRows nodes and maybe planning of set operations, and an inner function that would handle GROUP BY, DISTINCT, and possibly window function planning. For the reasons I mentioned, I'd like to get to a point where subquery_planner's output is Paths not Plans as soon as possible. But the idea of coarse representation of steps that we aren't trying to be smart about might be useful to save some labor in the short run. The zero-order version of that might be a single Path node type that represents do whatever grouping_planner would do, which we'd start to break down into multiple node types once we had the other APIs fixed. 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] Final Patch for GROUPING SETS
On 2015-05-13 22:51:15 +0200, Andres Freund wrote:
I'm pretty sure by now that I dislike the introduction of GroupedVar,
and not just tentatively. While I can see why you found its
introduction to be nicer than fiddling with the result tuple, for me the
disadvantages seem to outweigh the advantage. For one it's rather wierd
to have Var nodes be changed into GroupedVar in setrefs.c. The number
of places that need to be touched even when it's a 'planned stmt only'
type of node is still pretty large.
Andrew: I'll work on changing this in a couple hours unless you're
speaking up about doing it yourself.
I did a stab at removing it, and it imo definitely ends up looking
better.
The code for the GroupedVar replacement isn't perfect yet, but I think
it'd be possible to clean that up until Friday. Unfortunately, after
prolonged staring out of the window, I came to the conclusion that I
don't think the current tree structure isn't right.
I still believe that the general approach of chaining vs. a union or CTE
is correct due to the efficiency arguments upthread. My problem is
that, unless I very much misunderstand something, the current
implementation can end up requiring roughly #sets * #input of additional
space for the sidechannel tuplestore in some bad cases. That happens
if you group by a couple clauses that each lead to a high number of
groups.
That happens because the aggregated rows produced in the chain nodes
can't be returned up-tree, because the the next chain (or final group
aggregate) node will expect unaggregated tuples. The current solution
for that is to move the aggregated rows produced in chain nodes into a
tuplestore that's then drained when the top level aggregate node has
done it's own job.
While that's probably not too bad in many cases because most of the use
cases aggregation will be relatively effective, it does seem to be
further evidence that the sidechannel tuplestore isn't the perfect idea.
What I think we should/need to do instead is to the chaining locally
inside one aggregation node. That way the aggregated tuples can be
returned directly, without the sidechannel. While that will require
inlining part of the code from nodeSort.c it doesn't seem too bad.
Besides the advantage of getting rid of that tuplestore, it'll also fix
the explain performance problems (as there's no deep tree to traverse
via ruleutils.c), get rid of the the preemtive ExecReScan() to control
memory usage. I think it might also make combined hashing/sorting
easier.
A rough sketch of what I'm thinking of is:
ExecAgg()
{
...
while (!aggstate-consumed_input)
{
outerslot = ExecProcNode(outerPlanState(aggstate));
if (TupIsNull(outerslot))
{
consumed_input = true;
break;
}
if (aggstate-doing_hashing)
{
entry = lookup_hash_entry(aggstate, outerslot);
/* Advance the aggregates */
advance_aggregates(aggstate, entry-pergroup);
}
if (aggstate-presorted_input || AGG_PLAIN)
{
/* handle aggregation, return if done with group */
}
if (aggstate-doing_chaining)
{
tuplesort_puttupleslot(tuplesortstate, slot);
}
}
if (aggstate-doing_hashing !done)
agg_retrieve_hashed();
/*
* Feed data from one sort to the next, to compute grouping sets that
* need differing sort orders.
*/
last_sort = tuplesortstate[0];
current_sort = numGroupingSets 0 ? tuplesortstate[1] : NULL;
while (aggstate-doing_chaining !done_sorting)
{
tuplesort_gettupleslot(last_sort, tmpslot);
/* exhausted all tuple from a particular sort order, move on */
if (TupIsNull(tmpslot))
{
finalize_aggregates(...);
tuplesort_end(last_sort); /* maybe save stats somewhere? */
last_sort = current_sort;
current_sort = tuplesortstate[...];
if (all_sorts_done)
done_sorting = true;
return aggregated;
}
if (current_sort != NULL)
tuplesort_puttupleslot(current_sort, slot);
/* check if we crossed a boundary */
if (!execTuplesMatch(...))
{
finalize_aggregates(...);
aggstate-grp_firstTuple = ...
return aggregated;
}
advance_aggregates();
tuplesort_puttupleslot(current_sort, slot);
}
}
I think this is quite doable and seems likely to actually end up with
easier to understand code. But unfortunately it seems to be big enough
of a change to make it unlikely to be done in sufficient quality until
the freeze. I'll nonetheless work a couple hours on it tomorrow.
Andrew, is that a structure you could live with, or not?
Others, what do you think?
Greetings,
Andres Freund
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
Re: [HACKERS] subxcnt defined as signed integer in SnapshotData and SerializeSnapshotData
On Wed, May 13, 2015 at 12:17 PM, Michael Paquier michael.paqu...@gmail.com wrote: On Thu, May 14, 2015 at 1:03 AM, Michael Paquier wrote: On Thu, May 14, 2015 at 12:02 AM, Robert Haas wrote: I'm just going to remove the useless assertion for now. What you're proposing here may (or may not) be worth doing, but it carries a non-zero risk of breaking something somewhere, if anyone is relying on the signed-ness of that type. Removing the assertion is definitely safe. Fine for me. That's indeed possible for an extension. Btw, I think that your commit message should have given some credit to Coverity for finding the problem. Not a big deal though. The first report I received was from Andres via IM, actually: it showed up as a compiler warning for him. I just didn't get around to doing anything about it before this one showed up. I could have explained all that in the commit message, but for a one-line change, it didn't quite seem worth having 3 or 4 lines to attribute credit. -- 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] Triaging the remaining open commitfest items
Andres Freund and...@anarazel.de writes: On 2015-05-13 11:38:27 -0400, Tom Lane wrote: * parallel mode/contexts Robert's patch, his to deal with (likewise for assessing parallel-safety). Just as a note, a large part of this has been committed. Right, and Robert commented that he isn't planning to do more here for 9.5, so probably these CF items should be closed as committed? 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] Triaging the remaining open commitfest items
On 2015-05-13 11:38:27 -0400, Tom Lane wrote: * parallel mode/contexts Robert's patch, his to deal with (likewise for assessing parallel-safety). Just as a note, a large part of this has been committed. -- 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] Triaging the remaining open commitfest items
On Wed, May 13, 2015 at 1:06 PM, Tom Lane t...@sss.pgh.pa.us wrote: Andres Freund and...@anarazel.de writes: On 2015-05-13 11:38:27 -0400, Tom Lane wrote: * parallel mode/contexts Robert's patch, his to deal with (likewise for assessing parallel-safety). Just as a note, a large part of this has been committed. Right, and Robert commented that he isn't planning to do more here for 9.5, so probably these CF items should be closed as committed? Already done. -- 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] EvalPlanQual behaves oddly for FDW queries involving system columns
Etsuro Fujita fujita.ets...@lab.ntt.co.jp writes: On 2015/05/13 6:22, Tom Lane wrote: Of course, if we don't do that then we still have your original gripe about ctid not being correct in EvalPlanQual results. I poked at this a bit and it seems like we could arrange to pass ctid through even in the ROW_MARK_COPY case, if we define the t_ctid field of a composite Datum as being the thing to use. I did the same thing when creating the first version of the patch [1]. In addition, I made another change to ForeignNext so that the FDWs to get ctid coming out as the same value (0, 0) instead of (4294967295,0) before and after an EvalPlanQual recheck. But IIRC, I think both of them were rejected by you ... Ah, right. AFAIR, people objected to the other things you'd done in that patch, and I'd still say that the change in nodeForeignscan.c is unnecessary. But the idea of using t_ctid to solve the problem for the ROW_MARK_COPY code path seems reasonable enough. We could fix that by adding an ItemPointerSetInvalid((td-t_ctid)) call to heap_form_tuple, but I dunno if we want to add even a small number of cycles for this purpose to such a core function. I thought so too when creating the first version. On the other hand, that would only be three additional instructions on typical machines, which is surely in the noise compared to the rest of heap_form_tuple, and you could argue that it's a bug fix: leaving the t_ctid field with an apparently valid value is not very appropriate. So after thinking about it I think we ought to do that. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Streaming replication and WAL archive interactions
On 05/13/2015 04:29 PM, Robert Haas wrote: On Wed, May 13, 2015 at 8:53 AM, Heikki Linnakangas hlinn...@iki.fi wrote: Our manual says that archive_command should refuse to overwrite an existing file. But to work-around the double-archival problem, where the same file is archived twice, it would be even better if it would simply return success if the file exists, *and has identical contents*. I don't know how to code that logic in a simple one-liner though. This is why we really, really need that pg_copy command that was proposed a while back. Yeah.. I took a step back and looked at the big picture again: If we just implement the always mode, and you have a pg_copy command or similar that handles duplicates correctly, you don't necessarily need the shared mode at all. You can just set archive_command='always', and have the master and standby archive to the same location. As long as the archive_command works correctly and is race-free, that should work. I cut back the patch to implement just the always mode. The shared mode might still make sense as a future patch, as I think it's easier to understand and has less strict requirements for the archive_command, but let's take one step at a time. So attached is a patch that just adds the always mode. This is pretty close to what Fujii submitted long ago. - Heikki From 71332900247a8c68a61fcf60782cb35cf662b756 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas heikki.linnakangas@iki.fi Date: Thu, 16 Apr 2015 14:40:24 +0300 Subject: [PATCH 1/1] Add archive_mode='always' option. In 'always' mode, the standby's WAL archive is taken to be separate from the primary's, and the standby independently archives all files it receives from the primary. Fujii Masao and me. --- doc/src/sgml/config.sgml | 13 +++-- doc/src/sgml/high-availability.sgml | 39 +++ src/backend/access/transam/xlog.c | 22 +-- src/backend/access/transam/xlogarchive.c | 5 +++- src/backend/postmaster/postmaster.c | 37 ++--- src/backend/replication/walreceiver.c | 10 +-- src/backend/utils/misc/guc.c | 21 --- src/backend/utils/misc/postgresql.conf.sample | 2 +- src/include/access/xlog.h | 13 +++-- 9 files changed, 133 insertions(+), 29 deletions(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 0d8624a..5549b7d 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -2521,7 +2521,7 @@ include_dir 'conf.d' variablelist varlistentry id=guc-archive-mode xreflabel=archive_mode - termvarnamearchive_mode/varname (typeboolean/type) + termvarnamearchive_mode/varname (typeenum/type) indexterm primaryvarnamearchive_mode/ configuration parameter/primary /indexterm @@ -2530,7 +2530,16 @@ include_dir 'conf.d' para When varnamearchive_mode/ is enabled, completed WAL segments are sent to archive storage by setting -xref linkend=guc-archive-command. +xref linkend=guc-archive-command. In addition to literaloff/, +to disable, there are two modes: literalon/, and +literalalways/. During normal operation, there is no +difference between the two modes, but when set to literalalways/ +the WAL archiver is enabled also during archive recovery or standby +mode. In literalalways/ mode, all files restored from the archive +or streamed with streaming replication will be archived (again). See +xref linkend=continuous-archiving-in-standby for details. + /para + para varnamearchive_mode/ and varnamearchive_command/ are separate variables so that varnamearchive_command/ can be changed without leaving archiving mode. diff --git a/doc/src/sgml/high-availability.sgml b/doc/src/sgml/high-availability.sgml index a17f555..e93b711 100644 --- a/doc/src/sgml/high-availability.sgml +++ b/doc/src/sgml/high-availability.sgml @@ -1220,6 +1220,45 @@ primary_slot_name = 'node_a_slot' /sect3 /sect2 + + sect2 id=continuous-archiving-in-standby + titleContinuous archiving in standby/title + + indexterm + primarycontinuous archiving/primary + secondaryin standby/secondary + /indexterm + + para + When continuous WAL archiving is used in a standby, there are two + different scenarios: the WAL archive can be shared between the primary + and the standby, or the standby can have its own WAL archive. When + the standby has its own WAL archive, set varnamearchive_mode/varname + to literalalways/literal, and the standby will call the archive + command for every WAL segment it receives, whether it's by restoring + from the archive or by streaming replication. The shared archive can + be handled similarly, but the archive_command should test if the file + being archived
Re: [HACKERS] a few thoughts on the schedule
On Wed, May 13, 2015 at 10:32:03AM -0400, Robert Haas wrote: We also need to start thinking about what happens after feature freeze. The CommitFest application currently lists a 2015-06 CommitFest which, according to previous practice, would be expected to start on the 15th of the month. Given where we are now, that seems entirely unrealistic. I am doubtful that we will be ready to ship a beta by mid-June, let alone begin a new development cycle. This is a very good analysis. I have been holding back my trivial new patches for 9.6 in hopes of setting a good example. However, all my stuff is new, while many of these other patches have waited their turn for review, and we are going to be unfair to submitters if we don't give them the attention they deserve --- that is always the tension we have at this time. We have three days left so I think we need committers to devote serious time, if possible, to helping us resolve as much as we can. If we start thinking about this on Friday, it is too late. -- 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] a few thoughts on the schedule
On Wed, May 13, 2015 at 11:40 AM, Bruce Momjian br...@momjian.us wrote: We have three days left so I think we need committers to devote serious time, if possible, to helping us resolve as much as we can. If we start thinking about this on Friday, it is too late. Let me put a finer point on this --- whatever gets pushed to 9.6 unreasonably will be a feature we don't have in 9.5 and will discourage future development. I know we can't do magic, but now is the time to try. And on the flip side, whatever is pushed to 9.6 will not create a stability issue in 9.5. -- 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] Triaging the remaining open commitfest items
On 2015-05-13 11:38:27 -0400, Tom Lane wrote: Looking at what remains open in the current commitfest: * fsync $PGDATA recursively at startup Andres is the reviewer of record on this one. He should either commit it if he feels it's ready, or bounce it to next CF if not. The more important part of this has been committed by Robert. The other part, while also important in my opinion, has by now slipped 9.5. I've moved it. * Manipulating complex types as non-contiguous structures in-memory This one's mine of course. I've been hoping to get more independent performance testing than it's gotten, but time grows short. I'm inclined to just go ahead and push it in. I'm a bit hesitant about performance regressions around it. And I'd obviously rather not see the macros but the inline version ;). But I think overall we're in a better position with it, than without. If it turns out to have bad edge cases performancewise, we can still turn it off in plpgsql without much problems. If we, preferrably, can't find a better solution for the performance problem. * Grouping Sets I had originally promised to be committer for this one, and still want to go look at it, but Robert's nearby message about not committing stuff in haste definitely seems to apply. This has been in a limbo for a very long time. I'm right now working with Andrew getting it into a a committable shape. I'm not 100% sure we'll succeed for 9.5, but it deserves a chance. If it doesn't make it into 9.6, I plan to get into 9.6 soon after the branch opens. * Additional role attributes Is this ready to commit? Stephen's call. Not yet ready in my opinion. Greetings, Andres Freund -- 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] Proposal : REINDEX xxx VERBOSE
On Thu, May 14, 2015 at 12:28 AM, Robert Haas robertmh...@gmail.com wrote: On Thu, May 7, 2015 at 6:55 PM, Sawada Masahiko sawada.m...@gmail.com wrote: Sorry, I forgot attach files. Review comments: - Customarily we use int, rather than uint8, for flags variables. I think we should do that here also. - reindex_index() emits a log message either way, but at DEBUG2 level without VERBOSE and at the INFO level with it. I think we should skip it altogether without VERBOSE. i.e. if (options REINDEXOPT_VERBOSE) ereport(...) - The errmsg() in that function should not end with a period. - The 000 patch makes a pointless whitespace change to tab-complete.c. - Instead of an enumerated type (ReindexOption) just use #define to define the flag value. Apart from those fairly minor issues, I think this looks pretty solid. Thank you for reviwing.. All fixed. Regards, --- Sawada Masahiko 000_reindex_verbose_v14.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] Proposal : REINDEX xxx VERBOSE
Uh, are we really using INFO to log this? I thought that was a discouraged level to use anymore. Why not NOTICE? Also, when multiple tables are reindexed, do we emit lines for each index, or only for each table? If we're going to emit a line for each index in the single-table mode, it seems more sensible to do the same for the multi-table forms also. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Why does contain_leaked_vars believe MinMaxExpr is safe?
On Wed, May 13, 2015 at 09:34:53AM -0400, Robert Haas wrote: On Tue, May 12, 2015 at 7:22 PM, Tom Lane t...@sss.pgh.pa.us wrote: MinMaxExpr is an implicit invocation of a btree comparison function. Are we supposing that all of those are necessarily leakproof? I suspect it's an oversight, because the comment gives no hint that any such intention was present. It's been more than three years since I committed that code (under a different function name) so my memory is a little fuzzy, but I believe it just didn't occur to me that MinMaxExpr could include a function call. I suspect it's safe in practice, but in theory it's probably a bug. Agreed; it is formally a bug. We considered[1] special trust of operator class members and decided against it. Since almost every btree opfamily member is leakproof in practice, I doubt the bug has harmed anyone. [1] http://www.postgresql.org/message-id/flat/20110707223526.gj1...@tornado.leadboat.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: [COMMITTERS] pgsql: Map basebackup tablespaces using a tablespace_map file
On Thu, May 14, 2015 at 2:10 AM, Andrew Dunstan and...@dunslane.net wrote: How about if we simply abort if we find a non-symlink where we want the symlink to be, and only remove something that is actually a symlink (or a junction point, which is more or less the same thing)? We can do that way and for that I think we need to use rmdir instead of rmtree in the code being discussed (recovery path), OTOH we should try to minimize the errors raised during recovery. Then it would be up to the user to recover the situation, by moving or removing the offending file/directory, and trying again. Yes, I think as we only create/maintain symlinks in pg_tblspc for tablespaces, so it seems okay even if we error out when we find directories instead of symlinks in that path. I can write a patch for this if you, Heikki and or others think that is the better way to deal with this case. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
