Re: [HACKERS] Securing make check (CVE-2014-0067)
Re: Noah Misch 2014-03-24 20140323230420.ga4139...@tornado.leadboat.com
On Thu, Mar 06, 2014 at 11:52:22PM -0500, Noah Misch wrote:
On Thu, Mar 06, 2014 at 12:44:34PM -0500, Tom Lane wrote:
I'm inclined to suggest that we should put the socket under $CWD by
default, but provide some way for the user to override that choice.
If they want to put it in /tmp, it's on their head as to how secure
that is. On most modern platforms it'd be fine.
Fwiw, to relocate the pg_regress socket dir, there is already the
possibility to run make check EXTRA_REGRESS_OPTS=--host=/tmp. (With
the pending fix I sent yesterday to extend this to contrib/test_decoding.)
I am skeptical about the value of protecting systems with non-sticky /tmp,
but
long $CWD isn't of great importance, either. I'm fine with your suggestion.
Though the $CWD or one of its parents could be world-writable, that would
typically mean an attacker could just replace the test cases directly.
Here's the patch. The temporary data directory makes for a convenient socket
directory; initdb already gives it mode 0700, and we have existing
arrangements to purge it when finished. One can override the socket directory
by defining PG_REGRESS_SOCK_DIR in the environment.
We've been putting a small patch into pg_upgrade in Debian to work
around too long socket paths generated by pg_upgrade during running
the testsuite (and effectively on end user systems, but I don't think
anyone is using such long paths there).
A similar code bit could be put into pg_regress itself.
Fix for: connection to database failed: Unix-domain socket path
/build/buildd-postgresql-9.3_9.3~beta1-1-i386-mHjRUH/postgresql-9.3-9.3~beta1/build/contrib/pg_upgrade/.s.PGSQL.50432
is too long (maximum 107 bytes)
See also: http://lists.debian.org/debian-wb-team/2013/05/msg00015.html
--- a/contrib/pg_upgrade/option.c
+++ b/contrib/pg_upgrade/option.c
@@ -422,6 +422,11 @@ get_sock_dir(ClusterInfo *cluster, bool
cluster-sockdir = pg_malloc(MAXPGPATH);
if (!getcwd(cluster-sockdir, MAXPGPATH))
pg_fatal(cannot find current directory\n);
+#ifndef UNIX_PATH_MAX
+#define UNIX_PATH_MAX 108
+#endif
+ if (strlen(cluster-sockdir) UNIX_PATH_MAX -
sizeof(.s.PGSQL.50432))
+ strcpy(cluster-sockdir, /tmp); /* fall back
to tmp */
}
else
{
I had proposed that patch here before, but iirc it was rejected on
grounds of not a PostgreSQL problem.
Christoph
--
c...@df7cb.de | http://www.df7cb.de/
signature.asc
Description: Digital signature
Re: [HACKERS] psql \d+ and oid display
On Mar 28, 2014, at 2:41 PM, Tom Lane t...@sss.pgh.pa.us wrote: Bruce Momjian br...@momjian.us writes: On Thu, Mar 27, 2014 at 02:54:26PM -0400, Stephen Frost wrote: I believe Bruce was suggesting to show it when it is set to *not* the default, which strikes me as perfectly reasonable. We seem to be split on the idea of having Has OIDs display only when the oid status of the table does not match the default_with_oids default. FWIW, I think that having the display depend on what that GUC is set to is a seriously *bad* idea. It will mean that you don't actually know, when looking at the output of \d, whether the table has OIDs or not. Agreed. I could get behind a proposal to suppress the line when there are not OIDs, full stop; that is, we print either Has OIDs: yes or nothing. But I think this patch just makes things even more surprising when default_with_oids is turned on. I see little reason to tinker with the status quo. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] psql \d+ and oid display
On Fri, Mar 28, 2014 at 03:53:32PM -0300, FabrÃzio de Royes Mello wrote:
On Fri, Mar 28, 2014 at 3:41 PM, Tom Lane t...@sss.pgh.pa.us wrote:
Bruce Momjian br...@momjian.us writes:
On Thu, Mar 27, 2014 at 02:54:26PM -0400, Stephen Frost wrote:
I believe Bruce was suggesting to show it when it is set to *not* the
default, which strikes me as perfectly reasonable.
We seem to be split on the idea of having Has OIDs display only when
the oid status of the table does not match the default_with_oids
default.
FWIW, I think that having the display depend on what that GUC is set to
is a seriously *bad* idea. It will mean that you don't actually know,
when looking at the output of \d, whether the table has OIDs or not.
I could get behind a proposal to suppress the line when there are not
OIDs, full stop; that is, we print either Has OIDs: yes or nothing.
But I think this patch just makes things even more surprising when
default_with_oids is turned on.
Something like the attached ?
I assume it would be more like my attachment, i.e. since we are only
displaying it when OIDs exist, there is no value for oid status field
--- just say Has OIDs or Includes OIDs, or something like that.
I know some people are saying there is no need to change the current
output --- I am only saying that the importance of showing the lack of
OIDs has lessened over the years, and we should reconsider its
importance. If we reconsider and still think we are fine, that's good
with me. I am saying we should not just keep doing this because we have
always displayed it in the past.
--
Bruce Momjian br...@momjian.ushttp://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
new file mode 100644
index 21bbdf8..a5bf5c9
*** a/src/bin/psql/describe.c
--- b/src/bin/psql/describe.c
*** describeOneTableDetails(const char *sche
*** 2362,2375
}
/* OIDs, if verbose and not a materialized view */
! if (verbose tableinfo.relkind != 'm')
! {
! const char *s = _(Has OIDs);
!
! printfPQExpBuffer(buf, %s: %s, s,
! (tableinfo.hasoids ? _(yes) : _(no)));
! printTableAddFooter(cont, buf.data);
! }
/* Tablespace info */
add_tablespace_footer(cont, tableinfo.relkind, tableinfo.tablespace,
--- 2362,2369
}
/* OIDs, if verbose and not a materialized view */
! if (verbose tableinfo.relkind != 'm' tableinfo.hasoids)
! printTableAddFooter(cont, _(Has OIDs));
/* Tablespace info */
add_tablespace_footer(cont, tableinfo.relkind, tableinfo.tablespace,
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] psql \d+ and oid display
Bruce Momjian wrote On Fri, Mar 28, 2014 at 03:53:32PM -0300, FabrÃzio de Royes Mello wrote: On Fri, Mar 28, 2014 at 3:41 PM, Tom Lane lt; tgl@.pa gt; wrote: Bruce Momjian lt; bruce@ gt; writes: On Thu, Mar 27, 2014 at 02:54:26PM -0400, Stephen Frost wrote: I believe Bruce was suggesting to show it when it is set to *not* the default, which strikes me as perfectly reasonable. We seem to be split on the idea of having Has OIDs display only when the oid status of the table does not match the default_with_oids default. FWIW, I think that having the display depend on what that GUC is set to is a seriously *bad* idea. It will mean that you don't actually know, when looking at the output of \d, whether the table has OIDs or not. I could get behind a proposal to suppress the line when there are not OIDs, full stop; that is, we print either Has OIDs: yes or nothing. But I think this patch just makes things even more surprising when default_with_oids is turned on. Something like the attached ? I assume it would be more like my attachment, i.e. since we are only displaying it when OIDs exist, there is no value for oid status field --- just say Has OIDs or Includes OIDs, or something like that. I know some people are saying there is no need to change the current output --- I am only saying that the importance of showing the lack of OIDs has lessened over the years, and we should reconsider its importance. If we reconsider and still think we are fine, that's good with me. I am saying we should not just keep doing this because we have always displayed it in the past. As my belief is that 99% of the uses of \d are for human consumption (because machines should in most cases hit the catalogs directly) then strictly displaying Includes OIDs when appropriate has my +1. Uses of \d+ in regression suites will be obvious and quickly fixed and likely account for another 0.9%. psql backslash commands are not machine API contracts and should be adapted for optimal human consumption; thus neutering the argument for maintaining backward compatibility. David J. -- View this message in context: http://postgresql.1045698.n5.nabble.com/psql-d-and-oid-display-tp5797653p5797879.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.com. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] separate output dirs for test decoding pieces.
make check in contrib/test_decoding actually does two regression runs, one with pg_regress and one with pg_isolation_regress. These both use the same (default) outputdir, so one overwrites the other, which is a bit unfortunate from the buildfarm's point of view. I propose to make them use separate outputdirs, via the attached small patch. Comments? cheers andrew diff --git a/contrib/test_decoding/Makefile b/contrib/test_decoding/Makefile index c193f73..d6e6a6b 100644 --- a/contrib/test_decoding/Makefile +++ b/contrib/test_decoding/Makefile @@ -5,7 +5,7 @@ OBJS = test_decoding.o # Note: because we don't tell the Makefile there are any regression tests, # we have to clean those result files explicitly -EXTRA_CLEAN = -r $(pg_regress_clean_files) +EXTRA_CLEAN = $(pg_regress_clean_files) ./regression_output ./isolation_output ifdef USE_PGXS PG_CONFIG = pg_config @@ -40,10 +40,12 @@ submake-test_decoding: REGRESSCHECKS=ddl rewrite toast permissions decoding_in_xact binary regresscheck: all | submake-regress submake-test_decoding + $(MKDIR_P) regression_output $(pg_regress_check) \ --temp-config $(top_srcdir)/contrib/test_decoding/logical.conf \ --temp-install=./tmp_check \ --extra-install=contrib/test_decoding \ + --outputdir=./regress_output \ $(REGRESSCHECKS) regresscheck-install-force: | submake-regress submake-test_decoding @@ -54,9 +56,11 @@ regresscheck-install-force: | submake-regress submake-test_decoding ISOLATIONCHECKS=mxact delayed_startup concurrent_ddl_dml isolationcheck: all | submake-isolation submake-test_decoding + $(MKDIR_P) isolation_output $(pg_isolation_regress_check) \ --temp-config $(top_srcdir)/contrib/test_decoding/logical.conf \ --extra-install=contrib/test_decoding \ + --outputdir=./isolation_output \ $(ISOLATIONCHECKS) isolationcheck-install-force: all | submake-isolation submake-test_decoding -- 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] Composite Datums containing toasted fields are a bad idea(?)
Interesting bug. On Fri, Mar 28, 2014 at 04:34:41PM -0400, Tom Lane wrote: I think we might be better off to get rid of toast_flatten_tuple_attribute and instead insist that composite Datums never contain any external toast pointers in the first place. That is, places that call heap_form_tuple to make a composite Datum (rather than a tuple that's due to be stored to disk) would be responsible for detoasting any fields with external values first. We could make a wrapper routine for heap_form_tuple to take care of this rather than duplicating the code in lots of places. From a performance standpoint this is probably a small win. In cases where a composite Datum is formed and ultimately saved to disk, it should be a win, since we'd have to detoast those fields anyway, and we can avoid the overhead of an extra disassembly and reassembly of the composite value. If the composite value is never sent to disk, it's a bit harder to tell: we lose if the specific field value is never extracted from the composite, but on the other hand we win if it's extracted more than once. Performance is the dominant issue here; the hacker-centric considerations you outlined vanish next to it. Adding a speculative detoast can regress by a million-fold the performance of a query that passes around, without ever dereferencing, toast pointers to max-size values. Passing around a record without consulting all fields is a credible thing to do, so I'd scarcely consider taking the performance risk of more-aggressively detoasting every composite. That other cases win is little comfort. Today's PostgreSQL applications either suffer little enough to care or have already taken measures to avoid duplicate detoasts. Past discussions have examined general ways to avoid repetitive detoast traffic; we'll have something good when it can do that without imposing open-ended penalties on another usage pattern. Making the array construction functions use toast_flatten_tuple_attribute() carries a related performance risk, more elusive yet just as costly when encountered. That much risk seems tolerable for 9.4, though. I won't worry about performance regressions for range types; using a range to marshal huge values you'll never detoast is a stretch. In any case, adding the extra syscache lookups involved in doing toast_flatten_tuple_attribute in lots more places isn't appealing. True; alas. -- Noah Misch EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PQputCopyData dont signal error
I realize this is an old thread, but seems to be the only discussion I can find on this topic I have a problem with PQputCopyData function. It doesn't signal some error. I am using from within a c++ program: PQexec(m_pConn, COPY... ...FROM stdin), followed by PQputCopyData(m_pConn, ssQuery.str().c_str(), ssQuery.str().length()), finished with PQputCopyEnd(m_pConn, NULL). Everything that the gentleman that started this thread discussed is still in force in PostGres 9.3. Specifically that if some data anomaly within the data being copied causes the copy to fail, there is no notice, no return code change, and no way to know if this has happened until you examine the actual table being copied to itself and see that in fact the copy didn't work. Does anyone know of a way to find out if a copy went wrong so an error can be posted and the condition noted without having to check the table and count the records and compare the actual count against how many were expected to present after the copy. When one uses copy from the command line you get very useful feedback about the quality of your data such as: postgres=# COPY dr_cpdlc(id_pk, fk_guid_machine_run_info_lk, sim_time, wall_time,... Enter data to be copied followed by a newline. End with a backslash and a period on a line by itself. 988|4789|1394686027.05|1394686027.049000|ASTOR|RECV|ATOS|NASA02|4|47a|7... \. ERROR: invalid input syntax for integer: 47a CONTEXT: COPY dr_cpdlc, line 1, column minute_of_the_time: 47a postgres=# Does such error reporting functionality exist for using copy from within a c++ program? Does anyone have any ideas or know if this is being worked on? Am I completely missing something obvious here? If not I feel this is a feature shortcoming that could hopefully be addressed if it hasn't already. It prevents PostGres copy from being reliable for use when writing data during a live simulation run. Imagine getting approximately 100 or so people together including Air Traffic Controllers, Pilots, Researchers, Radio operators, and various other support staff. They spend all day (or days) running through various scenarios which generate say 20 frames a second of data for say a few hundred simulated aircraft plus all the simulated radio traffic and other com related data. After all these people have flown back to where they came from you notice some missing data. ---OOPs! :) Now what? What do you say to these people that put this experiment together?? Sorry? During these simulation runs, things break since each new scenario is testing a new hypothesis, and people expect to have to stop, recalibrate, tune/tweak code/settings, etc before the final run and actual data gathering. A data anomaly suddenly occurring would be a situation that all parties would want to resolve before going further. If you could capture this event (copy failing) and write it to a log, the source of the bad data could be addressed before going further. As things stand currently the vast reams of data might easily hide the fact that a few thousand or 10,000 records were missing until it was too late to do a rerun. Effectively, this precludes the use of PostGres in support of harvesting data during a simulation run unless someone knows a quicker way to jam data into a set of relational tables without falling out of realtime. Sorry to be so negative but I find it amazing that an rdbms engine as robust as PostGres seems to have this gaping hole in its capabilities and potential utilization. Coming from an Oracle world I was delighted to see the full functionality offered by PostGres that seems just as good and perhaps better (no listeners to troubleshoot, no pfile vs. spfile interactions from unusual shutdowns etc...) I haven't tried using inserts yet with multiple values clauses but have read this is possible. However, once the planner gets involved, speed drops noticeably. Thanks to everyone for their time or any feedback. Regards, Steve K. -- View this message in context: http://postgresql.1045698.n5.nabble.com/PQputCopyData-dont-signal-error-tp4302340p5797826.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.com. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PQputCopyData dont signal error
steve k steven.c.koh...@nasa.gov writes: I realize this is an old thread, but seems to be the only discussion I can find on this topic I have a problem with PQputCopyData function. It doesn't signal some error. PQputCopyData/PQputCopyEnd are only concerned with transferring data. After you're done with that, you should call PQgetResult to find out the COPY command's completion status. You should get PGRES_COMMAND_OK, otherwise it'll be an error result. Does anyone have any ideas or know if this is being worked on? Am I completely missing something obvious here? Um ... you could have read the manual: http://www.postgresql.org/docs/9.3/static/libpq-copy.html Or looked at the source code for psql, which as you noted has no problem displaying errors for COPY. 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] psql \d+ and oid display
On Sat, Mar 29, 2014 at 09:59:36AM -0700, David Johnston wrote: As my belief is that 99% of the uses of \d are for human consumption (because machines should in most cases hit the catalogs directly) then strictly displaying Includes OIDs when appropriate has my +1. Uses of \d+ in regression suites will be obvious and quickly fixed and likely account for another 0.9%. psql backslash commands are not machine API contracts and should be adapted for optimal human consumption; thus neutering the argument for maintaining backward compatibility. One other issue --- we are adding conditional display of Replica Identity to psql \d+ in 9.4, so users processing \d+ output are already going to have to make adjustments for 9.4. That is another reason I am asking about this now. -- 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] psql \d+ and oid display
On 03/29/2014 04:49 PM, Bruce Momjian wrote: On Sat, Mar 29, 2014 at 09:59:36AM -0700, David Johnston wrote: As my belief is that 99% of the uses of \d are for human consumption (because machines should in most cases hit the catalogs directly) then strictly displaying Includes OIDs when appropriate has my +1. Uses of \d+ in regression suites will be obvious and quickly fixed and likely account for another 0.9%. psql backslash commands are not machine API contracts and should be adapted for optimal human consumption; thus neutering the argument for maintaining backward compatibility. One other issue --- we are adding conditional display of Replica Identity to psql \d+ in 9.4, so users processing \d+ output are already going to have to make adjustments for 9.4. That is another reason I am asking about this now. I think Tom's suggestion probably has the most support, although it's not unanimous. 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] [RFC] What should we do for reliable WAL archiving?
On Fri, Mar 21, 2014 at 2:22 PM, MauMau maumau...@gmail.com wrote: From: Jeff Janes jeff.ja...@gmail.com Do people really just copy the files from one directory of local storage to another directory of local storage? I don't see the point of that. It makes sense to archive WAL to a directory of local storage for media recovery. Here, the local storage is a different disk drive which is directly attached to the database server or directly connected through SAN. For a SAN I guess we have different meanings of local :) (I have no doubt yours is correct--the fine art of IT terminology is not my thing.) The recommendation is to refuse to overwrite an existing file of the same name, and exit with failure. Which essentially brings archiving to a halt, because it keeps trying but it will keep failing. If we make a custom version, one thing it should do is determine if the existing archived file is just a truncated version of the attempting-to-be archived file, and if so overwrite it. Because if the first archival command fails with a network glitch, it can leave behind a partial file. What I'm trying to address is just an alternative to cp/copy which fsyncs a file. It just overwrites an existing file. Yes, you're right, the failed archive attempt leaves behind a partial file which causes subsequent attempts to fail, if you follow the PG manual. That's another undesirable point in the current doc. To overcome this, someone on this ML recommended me to do cp %p /archive/dir/%f.tmp mv /archive/dir/%f.tmp /archive/dir/%f. Does this solve your problem? As written is doesn't solve it, as it just unconditionally overwrites the file. If you wanted that you could just do the single-statement unconditional overwrite. You could make it so that the .tmp gets overwritten unconditionally, but the move of it will not overwrite an existing permanent file. That would solve the problem where a glitch in the network leaves in incomplete file behind that blocks the next attempt, *except* that mv on (at least some) network file systems is really a copy, and not an atomic rename, so is still subject to leaving behind incomplete crud. But, it is hard to tell what the real solution is, because the doc doesn't explain why it should refuse (and fail) to overwrite an existing file. The only reason I can think of to make that recommendation is because it is easy to accidentally configure two clusters to attempt to archive to the same location, and having them overwrite each others files should be guarded against. If I am right, it seems like this reason should be added to the docs, so people know what they are defending against. And if I am wrong, it seems even more important that the (correct) reason is added to the docs. Cheers, Jeff
Re: [HACKERS] [RFC] What should we do for reliable WAL archiving?
Jeff Janes jeff.ja...@gmail.com writes: But, it is hard to tell what the real solution is, because the doc doesn't explain why it should refuse (and fail) to overwrite an existing file. The only reason I can think of to make that recommendation is because it is easy to accidentally configure two clusters to attempt to archive to the same location, and having them overwrite each others files should be guarded against. If I am right, it seems like this reason should be added to the docs, so people know what they are defending against. And if I am wrong, it seems even more important that the (correct) reason is added to the docs. If memory serves, that is the reason ... and I thought it *was* explained somewhere in the docs. 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] psql \d+ and oid display
On Sat, Mar 29, 2014 at 05:10:49PM -0400, Andrew Dunstan wrote: On 03/29/2014 04:49 PM, Bruce Momjian wrote: On Sat, Mar 29, 2014 at 09:59:36AM -0700, David Johnston wrote: As my belief is that 99% of the uses of \d are for human consumption (because machines should in most cases hit the catalogs directly) then strictly displaying Includes OIDs when appropriate has my +1. Uses of \d+ in regression suites will be obvious and quickly fixed and likely account for another 0.9%. psql backslash commands are not machine API contracts and should be adapted for optimal human consumption; thus neutering the argument for maintaining backward compatibility. One other issue --- we are adding conditional display of Replica Identity to psql \d+ in 9.4, so users processing \d+ output are already going to have to make adjustments for 9.4. That is another reason I am asking about this now. I think Tom's suggestion probably has the most support, although it's not unanimous. Are you saying most people like Has OIDs: yes, or the idea of just displaying _a_ line if there are OIDs? Based on default_with_oids, perhaps we should display With OIDs. I agree it is no unanimous. I am curious how large the majority has to be to change a psql display value. -- 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] psql \d+ and oid display
On 03/29/2014 06:10 PM, Bruce Momjian wrote: On Sat, Mar 29, 2014 at 05:10:49PM -0400, Andrew Dunstan wrote: On 03/29/2014 04:49 PM, Bruce Momjian wrote: On Sat, Mar 29, 2014 at 09:59:36AM -0700, David Johnston wrote: As my belief is that 99% of the uses of \d are for human consumption (because machines should in most cases hit the catalogs directly) then strictly displaying Includes OIDs when appropriate has my +1. Uses of \d+ in regression suites will be obvious and quickly fixed and likely account for another 0.9%. psql backslash commands are not machine API contracts and should be adapted for optimal human consumption; thus neutering the argument for maintaining backward compatibility. One other issue --- we are adding conditional display of Replica Identity to psql \d+ in 9.4, so users processing \d+ output are already going to have to make adjustments for 9.4. That is another reason I am asking about this now. I think Tom's suggestion probably has the most support, although it's not unanimous. Are you saying most people like Has OIDs: yes, or the idea of just displaying _a_ line if there are OIDs? Based on default_with_oids, perhaps we should display With OIDs. I agree it is no unanimous. I am curious how large the majority has to be to change a psql display value. 1. _a_ line. 2. Don't make it dependent on the GUC. If the table has OIDS then say so, if not, say nothing. As to majority size, I have no idea. 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] psql \d+ and oid display
Bruce Momjian br...@momjian.us writes: Are you saying most people like Has OIDs: yes, or the idea of just displaying _a_ line if there are OIDs? Based on default_with_oids, perhaps we should display With OIDs. I agree it is no unanimous. I am curious how large the majority has to be to change a psql display value. What I actually suggested was not *changing* the line when it's to be displayed, but suppressing it in the now-standard case where there's no OIDs. Personally I find the argument that backwards compatibility must be preserved to be pretty bogus; we have no hesitation in changing the output of \d anytime we add a new feature. So I don't think there's a good compatibility reason why the line has to be spelled exactly Has OIDs: yes --- but there is a consistency reason, which is that everything else we print in this part of the \d output is of the form label: info. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: [COMMITTERS] pgsql: Revert Secure Unix-domain sockets of make check temporary clu
On Sat, Mar 29, 2014 at 01:48:33PM -0400, Andrew Dunstan wrote: On 03/29/2014 01:22 PM, Noah Misch wrote: http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=dromedarydt=2014-03-29%2007%3A02%3A48 Hmm. Can we use a location with a bit more head room than the tmp_check/data directory? Maybe something like src/test/sockets? Note that the buildfarm's buildroot (the part of the name before the branch name) is not terribly long in some of these cases. e.g. in the first case it's only 32 chars long. That's tempting, but I don't think freeing up ~25 bytes changes the verdict. Christoph brought up that Debian builds in directory trees deeper than those the buildfarm uses, and I suspect Debian is not alone. I think we're back looking at using a subdirectory of /tmp, with the open question being what properties (sticky bit, ownership, _PC_CHOWN_RESTRICTED), if any, to verify on /tmp and its parent(s) before proceeding. I looked around to see what other projects are doing. File::Temp is the one project I found that has an option[1], disabled by default, to security-check /tmp. Even OpenSSH simply assumes /tmp is suitable. Perhaps the threat of insecure /tmp has received less attention than it deserves, or perhaps secure /tmp is considered a mandatory component of a multi-user Unix system. In any event, I do not feel the need to put PostgreSQL make check in the vanguard concerning this issue. Assuming a secure /tmp, like OpenSSH does, is reasonable. -- Noah Misch EnterpriseDB http://www.enterprisedb.com [1] http://search.cpan.org/~dagolden/File-Temp-0.2304/lib/File/Temp.pm#safe_level -- 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] PQputCopyData dont signal error
steve k wrote I realize this is an old thread, but seems to be the only discussion I can find on this topic I have a problem with PQputCopyData function. It doesn't signal some error. I am using from within a c++ program: PQexec(m_pConn, COPY... ...FROM stdin), followed by PQputCopyData(m_pConn, ssQuery.str().c_str(), ssQuery.str().length()), finished with PQputCopyEnd(m_pConn, NULL). This may be over simplified but... To summarize here the PQexec needs a matching PQgetResult. The PQputCopyEnd only closes the preceding PQputCopyData. The server is not compelled to process the copy data until the client says that no more data is coming. Once the PQputCopyEnd has returned for practical purposes you back to the generic command handling API and need to proceed as you would for any other command - including being prepared to wait for long-running commands and handle errors. The request in this thread is for some means for the client to send partial data and if the server has chosen to process that data (or maybe the client can compel it to start...) AND has encountered an error then the client can simply abort the rest of the copy and return an error. The current API return values deal with the results of the actual call just performed and not with any pre-existing state on the server. Tom's suggestion is to add a polling method to query the current server state for those who care to expend the overhead instead of imposing that overhead on all callers. The C API seems strange to me, a non-C programmer, but at a cursory glance it is at least internally consistent and does provide lots of flexibility (especially for sync/async options) at the cost of complexity and having to make sure that you code in the appropriate PQgetResult checks or risk ignoring errors and reporting success incorrectly. David J. -- View this message in context: http://postgresql.1045698.n5.nabble.com/PQputCopyData-dont-signal-error-tp4302340p5797912.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.com. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] psql \d+ and oid display
On Sat, Mar 29, 2014 at 06:16:19PM -0400, Tom Lane wrote: Bruce Momjian br...@momjian.us writes: Are you saying most people like Has OIDs: yes, or the idea of just displaying _a_ line if there are OIDs? Based on default_with_oids, perhaps we should display With OIDs. I agree it is no unanimous. I am curious how large the majority has to be to change a psql display value. What I actually suggested was not *changing* the line when it's to be displayed, but suppressing it in the now-standard case where there's no OIDs. Personally I find the argument that backwards compatibility must be preserved to be pretty bogus; we have no hesitation in changing the output of \d anytime we add a new feature. So I don't think there's a good compatibility reason why the line has to be spelled exactly Has OIDs: yes --- but there is a consistency reason, which is that everything else we print in this part of the \d output is of the form label: info. Ah, now I understand it --- you can argue that the new Replica Identity follows the same pattern, showing only for non-defaults (or at least it will once I commit the pending patch to do that). -- 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] Useless Replica Identity: NOTHING noise from psql \d
On Thu, Mar 27, 2014 at 11:02:55AM -0400, Bruce Momjian wrote: On Thu, Mar 27, 2014 at 10:30:59AM -0400, Bruce Momjian wrote: On Thu, Mar 27, 2014 at 10:13:36AM -0400, Tom Lane wrote: Bruce Momjian br...@momjian.us writes: On Thu, Mar 27, 2014 at 02:41:40PM +0100, Christoph Berg wrote: I meant to say what's actually in git HEAD at the moment is broken. Uh, I thought that might be what you were saying, but I am not seeing any failures here. The buildfarm isn't complaining, either. Is there some part of make check-world that isn't exercised by the buildfarm? I see it now in contrib/test_decoding; fixing. OK, fixed. I have also updated my new patch to reflect those changes, attached. Applied. -- 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] [RFC] What should we do for reliable WAL archiving?
On Saturday, March 29, 2014, Tom Lane t...@sss.pgh.pa.us wrote: Jeff Janes jeff.ja...@gmail.com javascript:; writes: But, it is hard to tell what the real solution is, because the doc doesn't explain why it should refuse (and fail) to overwrite an existing file. The only reason I can think of to make that recommendation is because it is easy to accidentally configure two clusters to attempt to archive to the same location, and having them overwrite each others files should be guarded against. If I am right, it seems like this reason should be added to the docs, so people know what they are defending against. And if I am wrong, it seems even more important that the (correct) reason is added to the docs. If memory serves, that is the reason ... and I thought it *was* explained somewhere in the docs. You are right, and it has been there for a decade. I don't know how I missed that the last several times I read it. I remember clearly the paragraph below it, just not that one. Sorry, Jeff
Re: [HACKERS] Securing make check (CVE-2014-0067)
On Sat, Mar 29, 2014 at 10:04:55AM +0100, Christoph Berg wrote: Fwiw, to relocate the pg_regress socket dir, there is already the possibility to run make check EXTRA_REGRESS_OPTS=--host=/tmp. (With the pending fix I sent yesterday to extend this to contrib/test_decoding.) That doesn't work for make check, because the postmaster ends up with listen_addresses=/tmp. We've been putting a small patch into pg_upgrade in Debian to work around too long socket paths generated by pg_upgrade during running the testsuite (and effectively on end user systems, but I don't think anyone is using such long paths there). A similar code bit could be put into pg_regress itself. Thanks for reminding me about Debian's troubles here. Once the dust settles on pg_regress, it will probably make sense to do likewise to pg_upgrade. -- Noah Misch EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: Fwd: [HACKERS] Proposal: variant of regclass
On Sat, Mar 22, 2014 at 1:17 PM, Amit Kapila amit.kapil...@gmail.com wrote:
On Wed, Feb 26, 2014 at 11:56 AM, Yugo Nagata nag...@sraoss.co.jp wrote:
Thanks for your a lot of comments. I revised the patch according to
comments from Robert Haas and Marti Raudsepp.
I have started looking into this patch and below are my
initial findings:
I have looked further into this patch and below are some more findings.
This completes my review for this patch.
1.
regclass_guts(..)
{
..
if (HeapTupleIsValid(tuple = systable_getnext(sysscan)))
*classid_p = HeapTupleGetOid(tuple);
else
return false;
/* We assume there can be only one match */
systable_endscan(sysscan);
heap_close(hdesc, AccessShareLock);
}
In case tuple is not found, false is returned without closing the scan and
relation, now if error is returned it might be okay, because it will release
lock during abort, but if error is not returned (in case of to_regclass),
it will be considered as leak. I think it might not effect anything directly,
because we are not using to_regclass() in Bootstrap mode, but still I feel
it is better to avoid such a leak in API. We can handle it similar to how it
is done in regproc_guts(). The similar improvement is required in
regtype_guts().
2.
! OpernameGetCandidates(List *names, char oprkind, bool missing_schema_ok)
{
..
! namespaceId = LookupExplicitNamespace(schemaname, missing_schema_ok);
! if (!OidIsValid(namespaceId))
! return NULL;
}
In this check it is better to check missing_schema_ok along with
invalid oid check, before returning NULL.
3.
/*
* to_regproc - converts proname to proc OID
*
* Diffrence from regprocin is, this does not throw an error and returns NULL
* if the proname does not exist.
* Note: this is not an I/O function.
*/
I think function header comments can be improved for all (to_*) newly
added functions. You can refer function header comments for
simple_heap_insert
which explains it's difference from heap_insert.
4. Oid's used for newly added functions became duplicate after a recent checkin.
5. This file is divided into /* SUPPORT ROUTINES*/ and USER I/O ROUTINES
isn't it better to move _guts functions into Support Routines.
6.
! parseTypeString(const char *str, Oid *typeid_p, int32 *typmod_p,
bool missing_ok)
Line length greater than 80
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] MultiXactId error after upgrade to 9.3.4
Greetings, Looks like we might not be entirely out of the woods yet regarding MultiXactId's. After doing an upgrade from 9.2.6 to 9.3.4, we saw the following: ERROR: MultiXactId 6849409 has not been created yet -- apparent wraparound The table contents can be select'd out and match the pre-upgrade backup, but any attempt to VACUUM / VACUUM FULL / CLUSTER fails, unsurprisingly. I've just started looking into this, but here's a bit more data- The *NEW* (9.3.4) cluster's pg_multixact files: pg_multixact/members/ -rw--- 1 postgres postgres 8192 Mar 30 02:33 pg_multixact/offsets/ -rw--- 1 postgres postgres 8192 Mar 28 01:11 -rw--- 1 postgres postgres 122880 Mar 30 02:33 0018 The *OLD* (9.2.6) cluster's pg_multixact files: pg_multixact/members/ -rw-rw-r-- 1 postgres postgres 188416 Mar 30 03:07 0044 pg_multixact/offsets/ -rw-rw-r-- 1 postgres postgres 114688 Mar 30 03:07 0018 txid_current - 40297693 datfrozenxid - 654 autovacuum_freeze_max_age, vacuum_freeze_min_age, vacuum_freeze_table_age, multixact GUCs are commented out / default values. The *NEW* (9.3.4) control data shows: pg_control version number:937 Catalog version number: 201306121 Latest checkpoint's NextXID: 0/40297704 Latest checkpoint's NextOID: 53773598 Latest checkpoint's NextMultiXactId: 1601771 Latest checkpoint's NextMultiOffset: 1105 Latest checkpoint's oldestXID:654 Latest checkpoint's oldestXID's DB: 12036 Latest checkpoint's oldestActiveXID: 40297704 Latest checkpoint's oldestMultiXid: 1601462 Latest checkpoint's oldestMulti's DB: 0 The *OLD* (9.2.6) control data had: pg_control version number:922 Catalog version number: 201204301 Latest checkpoint's NextXID: 0/40195831 Latest checkpoint's NextOID: 53757891 Latest checkpoint's NextMultiXactId: 1601462 Latest checkpoint's NextMultiOffset: 4503112 Latest checkpoint's oldestXID:654 Latest checkpoint's oldestXID's DB: 12870 Latest checkpoint's oldestActiveXID: 0 (It doesn't report the oldestMulti info under 9.2.6). The pg_upgrade reported: Setting oldest multixact ID on new cluster While testing, I discovered that this didn't appear to happen with a (earlier) upgrade to 9.3.2. Don't know if it's indicative of anything. Here is what pageinspace shows for the record: -[ RECORD 1 ] lp | 45 lp_off | 5896 lp_flags| 1 lp_len | 50 t_xmin | 9663920 t_xmax | 6849409 t_field3| 26930 t_ctid | (0,45) t_infomask2 | 5 t_infomask | 6528 t_hoff | 24 t_bits | t_oid | Which shows xmax as 6849409 and HEAP_XMAX_IS_MULTI is set. This is the only tuple in the table which has HEAP_XMAX_IS_MULTI and the xmax for all of the other tuples is quite a bit higher (though all are visible currently). Thoughts? Thanks, Stephen signature.asc Description: Digital signature
