[HACKERS] Implementing parallel sequence doubts
Hi Hackers, Implementation of Parallel Sequence Scan Approach: 1.Parallel Sequence Scan can achieved by using the background workers doing the job of actual sequence scan including the qualification check also. 2. Planner generates the parallel scan plan by checking the possible criteria of the table to do a parallel scan and generates the tasks (range of blocks). 3. In the executor Init phase, Try to copy the necessary data required by the workers and start the workers. 4. In the executor run phase, just get the tuples which are sent by the workers and process them further in the plan node execution. some of the problems i am thinking: 1. Data structures that are required to be copied from backend to worker are currentTransactionState, Snapshot, GUC, ComboCID, Estate and etc. I see some problems in copying Estate data structure into the shared memory because it contains so many pointers. There is a need of some infrastructure to copy these data structures into the shared memory. If the backend takes care of locking the relation and there are no sub plans involved in the qual and targetlist, is the estate is really required in the worker to achieve the parallel scan? Is it possible to Initialize the qual, targetlist and projinfo with the local Estate structure created in the worker with the help of plan structure received from the backend? Or In case if estate is required in the worker for the processing, is there any better way possible to share backend data structures with the workers? Any suggestions? Regards, Hari Babu Fujitsu Australia -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP Patch for GROUPING SETS phase 1
On 08/13/2014 09:43 PM, Atri Sharma wrote: Sorry, forgot to attach the patch for fixing cube in contrib, which breaks since we now reserve cube keyword. Please find attached the same. Ugh, that will make everyone using the cube extension unhappy. After this patch, they will have to quote contrib's cube type and functions every time. I think we should bite the bullet and rename the extension, and its cube type and functions. For an application, having to suddenly quote it has the same effect as renaming it; you'll have to find all the callers and change them. And in the long-run, it's clearly better to have an unambiguous name. - 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] Parallel Sequence Scan doubts
Corrected subject. Hi Hackers, Implementation of Parallel Sequence Scan Approach: 1.Parallel Sequence Scan can achieved by using the background workers doing the job of actual sequence scan including the qualification check also. 2. Planner generates the parallel scan plan by checking the possible criteria of the table to do a parallel scan and generates the tasks (range of blocks). 3. In the executor Init phase, Try to copy the necessary data required by the workers and start the workers. 4. In the executor run phase, just get the tuples which are sent by the workers and process them further in the plan node execution. some of the problems i am thinking: 1. Data structures that are required to be copied from backend to worker are currentTransactionState, Snapshot, GUC, ComboCID, Estate and etc. I see some problems in copying Estate data structure into the shared memory because it contains so many pointers. There is a need of some infrastructure to copy these data structures into the shared memory. If the backend takes care of locking the relation and there are no sub plans involved in the qual and targetlist, is the estate is really required in the worker to achieve the parallel scan? Is it possible to Initialize the qual, targetlist and projinfo with the local Estate structure created in the worker with the help of plan structure received from the backend? Or In case if estate is required in the worker for the processing, is there any better way possible to share backend data structures with the workers? Any suggestions? Regards, Hari Babu Fujitsu Australia -- 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] TODO : Allow parallel cores to be used by vacuumdb [ WIP ]
On 21 August 2014 08:31, Amit Kapila Wrote, Not sure. How about *concurrent* or *multiple*? multiple isn't right, but we could say concurrent. I also find concurrent more appropriate. Dilip, could you please change it to concurrent in doc updates, variables, functions unless you see any objection for the same. Ok, I will take care along with other comments fix.. Regards, Dilip Kumar
Re: [HACKERS] pg_xlogdump --stats
On 07/07/2014 10:32 PM, Abhijit Menon-Sen wrote: At 2014-07-07 09:48:44 +0200, and...@2ndquadrant.com wrote: I'd suggest only defining INT64_MODIFIER here and build INT64_FORMAT, UINT64_FORMAT ontop, in c.h. Oh, I see. I'm sorry, I misread your earlier suggestion. Regenerated patches attached. Is this what you had in mind? Committed the patch to add INT64_MODIFIER, with minor fixes. The new rm_identify method needs to be documented. Not sure where; surprisingly I can't find any documentation on the existing methods either. Perhaps add comments to the RmgrData struct, explaining all of the methods. In particular, should give guidelines on what output belongs in rm_desc and what in rm_identify. I think the names that rm_identify returns should match those that the rm_desc functions print. As the patch stands, for example when heap_desc prints out something like: hot_update(init): xmax 123; new tid 1/2 xmax 456 The corresponding rm_identify output is: HOT_UPDATE+INIT It would be better to spell them the same, so that when you look at the summary stats and the raw pg_xlogdump output, you can tell which records contributed to which summary line. - 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] Specifying the unit in storage parameter
On Fri, Aug 8, 2014 at 12:32 PM, Fujii Masao masao.fu...@gmail.com wrote: This is not user-friendly. I'd like to propose the attached patch which introduces the infrastructure which allows us to specify the unit when setting INTEGER storage parameter like autovacuum_vacuum_cost_delay. Comment? Review? This patch makes autovacuum_vacuum_cost_delay more consistent with what is at server level. So +1. Looking at the patch, the parameter fillfactor in the category RELOPT_KIND_HEAP (the first element in intRelOpts of reloptions.c) is not updated with the new field. It is only a one-line change. @@ -97,7 +97,7 @@ static relopt_int intRelOpts[] = Packs table pages only to this percentage, RELOPT_KIND_HEAP }, - HEAP_DEFAULT_FILLFACTOR, HEAP_MIN_FILLFACTOR, 100 + HEAP_DEFAULT_FILLFACTOR, HEAP_MIN_FILLFACTOR, 100, 0 }, Except that, I tested as well the patch and it works as expected. The documentation, as well as the regression tests remain untouched, but I guess that this is fine (not seeing similar tests in regressions, and documentation does not specify the unit for a given parameter). Regards, -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP Patch for GROUPING SETS phase 1
Heikki == Heikki Linnakangas hlinnakan...@vmware.com writes: On 08/13/2014 09:43 PM, Atri Sharma wrote: Sorry, forgot to attach the patch for fixing cube in contrib, which breaks since we now reserve cube keyword. Please find attached the same. Heikki Ugh, that will make everyone using the cube extension Heikki unhappy. After this patch, they will have to quote contrib's Heikki cube type and functions every time. Heikki I think we should bite the bullet and rename the extension, I agree, the contrib/cube patch as posted is purely so we could test everything without having to argue over the new name first. (And it is posted separately from the main patch because of its length and utter boringness.) However, even if/when a new name is chosen, there's the question of how to make the upgrade path easiest. Once CUBE is reserved, up-to-date pg_dump will quote all uses of the cube type and function when dumping an older database (except inside function bodies of course), so there may be merit in keeping a cube domain over the new type, and maybe also merit in keeping the extension name. So what's the new type name going to be? cuboid? hypercube? geometric_cube? n_dimensional_box? -- Andrew (irc:RhodiumToad) -- 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] [TODO] Process pg_hba.conf keywords as case-insensitive
On 07/23/2014 09:14 AM, Viswanatham kirankumar wrote: On 16 July 2014 23:12, Tom Lane wrote Christoph Berg c...@df7cb.de writes: Re: Viswanatham kirankumar 2014-07-16 ec867def52699d4189b584a14baa7c2165440...@blreml504-mbx.china.huawei.com Attached patch is implementing following TODO item Process pg_hba.conf keywords as case-insensitive Hmm. I see a case for accepting ALL (as in hosts.allow(5)), so +1 on that, but I don't think the other keywords like host and peer should be valid in upper case. I think the argument was that SQL users are accustomed to thinking that keywords are case-insensitive. It makes sense to me that we should adopt that same convention in pg_hba.conf. Re-reading the original thread, there was also concern about whether we should try to make quoting/casefolding behave more like it does in SQL, specifically for matching pg_hba.conf items to SQL identifiers (database and role names). This patch doesn't seem to have addressed that part of it, but I think we need to think those things through before we just do a blind s/strcmp/pg_strcasecmp/g. Otherwise we might find that we've added ambiguity that will give us trouble when we do try to fix that. I had updated as per you review comments 1) database and role names behave similar to SQL identifiers (case-sensitive / case-folding). 2) users and user-groups only requires special handling and behavior as follows Normal user : A. unquoted ( USER ) will be treated as user ( downcase ). B. quoted ( USeR ) will be treated as USeR (case-sensitive). C. quoted ( +USER ) will be treated as normal user +USER (i.e. will not be considered as user-group) and case-sensitive as string is quoted. User Group : A. unquoted ( +USERGROUP ) will be treated as +usergruop ( downcase ). B. plus quoted ( +UserGROUP ) will be treated as +UserGROUP (case-sensitive). 3) Host name is not a SQL object so it will be treated as case-sensitive except for all, samehost, samenet are considered as keywords. For these user need to use quotes to differentiate between hostname and keywords. 4) All the fixed keywords mention in pg_hba.conf and Client Authentication section will be considered as keywords Eg: host, local, hostssl etc.. With this patch, database (and role?) names are compared case-insensitively. For example: local MixedDB all trust local mixedDB all reject psql -d mixedDB psql (9.5devel) Type help for help. mixedDB=# That connection should've matched that 2nd line, and be rejected. PS. Please update the docs. - 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] 9.5: Memory-bounded HashAgg
On Wed, 2014-08-20 at 14:32 -0400, Robert Haas wrote: Well, I think you're certainly right that a hash table lookup is more expensive than modulo on a 32-bit integer; so much is obvious. But if the load factor is not too large, I think that it's not a *lot* more expensive, so it could be worth it if it gives us other advantages. As I see it, the advantage of Jeff's approach is that it doesn't really matter whether our estimates are accurate or not. We don't have to decide at the beginning how many batches to do, and then possibly end up using too much or too little memory per batch if we're wrong; we can let the amount of memory actually used during execution determine the number of batches. That seems good. Of course, a hash join can increase the number of batches on the fly, but only by doubling it, so you might go from 4 batches to 8 when 5 would really have been enough. And a hash join also can't *reduce* the number of batches on the fly, which might matter a lot. Getting the number of batches right avoids I/O, which is a lot more expensive than CPU. My approach uses partition counts that are powers-of-two also, so I don't think that's a big differentiator. In principle my algorithm could adapt to other partition counts, but I'm not sure how big of an advantage there is. I think the big benefit of my approach is that it doesn't needlessly evict groups from the hashtable. Consider input like 0, 1, 0, 2, ..., 0, N. For large N, if you evict group 0, you have to write out about N tuples; but if you leave it in, you only have to write out about N/2 tuples. The hashjoin approach doesn't give you any control over eviction, so you only have about 1/P chance of saving the skew group (where P is the ultimate number of partitions). With my approach, we'd always keep the skew group in memory (unless we're very unlucky, and the hash table fills up before we even see the skew value). Regards, Jeff Davis -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] [TODO] Track number of files ready to be archived in pg_stat_archiver
Hello,
Attached patch implements the following TODO item :
Track number of WAL files ready to be archived in pg_stat_archiver
However, it will track the total number of any file ready to be
archived, not only WAL files.
Please let me know what you think about it.
Regards.
--
Julien Rouhaud
http://dalibo.com - http://dalibo.org
*** a/doc/src/sgml/monitoring.sgml
--- b/doc/src/sgml/monitoring.sgml
***
*** 728,733 postgres 27093 0.0 0.0 30096 2752 ?Ss 11:34 0:00 postgres: ser
--- 728,738
entryTime of the last failed archival operation/entry
/row
row
+ entrystructfieldready_count//entry
+ entrytypebigint/type/entry
+ entryNumber of files waiting to be archived/entry
+ /row
+ row
entrystructfieldstats_reset//entry
entrytypetimestamp with time zone/type/entry
entryTime at which these statistics were last reset/entry
*** a/src/backend/access/transam/xlogarchive.c
--- b/src/backend/access/transam/xlogarchive.c
***
*** 24,29
--- 24,30
#include access/xlog_internal.h
#include miscadmin.h
#include postmaster/startup.h
+ #include pgstat.h
#include replication/walsender.h
#include storage/fd.h
#include storage/ipc.h
***
*** 539,544 XLogArchiveNotify(const char *xlog)
--- 540,548
/* Notify archiver that it's got something to do */
if (IsUnderPostmaster)
SendPostmasterSignal(PMSIGNAL_WAKEN_ARCHIVER);
+
+ /* Tell the collector about a new file waiting to be archived */
+ pgstat_send_archiver(xlog, ARCH_READY);
}
/*
*** a/src/backend/catalog/system_views.sql
--- b/src/backend/catalog/system_views.sql
***
*** 697,702 CREATE VIEW pg_stat_archiver AS
--- 697,703
s.failed_count,
s.last_failed_wal,
s.last_failed_time,
+ s.ready_count,
s.stats_reset
FROM pg_stat_get_archiver() s;
*** a/src/backend/postmaster/pgarch.c
--- b/src/backend/postmaster/pgarch.c
***
*** 491,497 pgarch_ArchiverCopyLoop(void)
* Tell the collector about the WAL file that we successfully
* archived
*/
! pgstat_send_archiver(xlog, false);
break; /* out of inner retry loop */
}
--- 491,497
* Tell the collector about the WAL file that we successfully
* archived
*/
! pgstat_send_archiver(xlog, ARCH_SUCCESS);
break; /* out of inner retry loop */
}
***
*** 501,507 pgarch_ArchiverCopyLoop(void)
* Tell the collector about the WAL file that we failed to
* archive
*/
! pgstat_send_archiver(xlog, true);
if (++failures = NUM_ARCHIVE_RETRIES)
{
--- 501,507
* Tell the collector about the WAL file that we failed to
* archive
*/
! pgstat_send_archiver(xlog, ARCH_FAIL);
if (++failures = NUM_ARCHIVE_RETRIES)
{
*** a/src/backend/postmaster/pgstat.c
--- b/src/backend/postmaster/pgstat.c
***
*** 36,41
--- 36,42
#include access/transam.h
#include access/twophase_rmgr.h
#include access/xact.h
+ #include access/xlog_internal.h
#include catalog/pg_database.h
#include catalog/pg_proc.h
#include lib/ilist.h
***
*** 3084,3094 pgstat_send(void *msg, int len)
* pgstat_send_archiver() -
*
* Tell the collector about the WAL file that we successfully
! * archived or failed to archive.
* --
*/
void
! pgstat_send_archiver(const char *xlog, bool failed)
{
PgStat_MsgArchiver msg;
--- 3085,3096
* pgstat_send_archiver() -
*
* Tell the collector about the WAL file that we successfully
! * archived or failed to archive, or the new file waiting
! * to be archived.
* --
*/
void
! pgstat_send_archiver(const char *xlog, ArchiverReason reason)
{
PgStat_MsgArchiver msg;
***
*** 3096,3102 pgstat_send_archiver(const char *xlog, bool failed)
* Prepare and send the message
*/
pgstat_setheader(msg.m_hdr, PGSTAT_MTYPE_ARCHIVER);
! msg.m_failed = failed;
strncpy(msg.m_xlog, xlog, sizeof(msg.m_xlog));
msg.m_timestamp = GetCurrentTimestamp();
pgstat_send(msg, sizeof(msg));
--- 3098,3104
* Prepare and send the message
*/
pgstat_setheader(msg.m_hdr, PGSTAT_MTYPE_ARCHIVER);
! msg.m_reason = reason;
strncpy(msg.m_xlog, xlog, sizeof(msg.m_xlog));
msg.m_timestamp = GetCurrentTimestamp();
pgstat_send(msg, sizeof(msg));
***
*** 3921,3927 pgstat_read_statsfiles(Oid onlydb, bool permanent, bool deep)
/*
* Try to open the stats file. If it doesn't exist, the backends simply
* return zero for anything and the collector simply starts from scratch
! * with empty counters.
*
* ENOENT is a possibility if the stats collector is not running or has
* not yet written the stats file the first time. Any other failure
--- 3923,3930
Re: [HACKERS] [GSoC2014] Patch ALTER TABLE ... SET LOGGED
Re: Thom Brown 2014-08-20 CAA-aLv7TeF8iM=7u7tsgl4s5jh1a+shq_ny7gorzc_g_yj7...@mail.gmail.com ERROR: table test is not permanent Perhaps this would be better as table test is unlogged as permanent doesn't match the term used in the DDL syntax. I was also wondering that, but then figured that when ALTER TABLE SET UNLOGGED is invoked on temp tables, the error message is not permanent was correct while the apparent opposite is unlogged is wrong. Christoph -- c...@df7cb.de | http://www.df7cb.de/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [TODO] Process pg_hba.conf keywords as case-insensitive
Re: Heikki Linnakangas 2014-08-21 53f5a2d6.2050...@vmware.com 1) database and role names behave similar to SQL identifiers (case-sensitive / case-folding). 2) users and user-groups only requires special handling and behavior as follows Normal user : A. unquoted ( USER ) will be treated as user ( downcase ). B. quoted ( USeR ) will be treated as USeR (case-sensitive). With this patch, database (and role?) names are compared case-insensitively. For example: local MixedDB all trust local mixedDB all reject psql -d mixedDB psql (9.5devel) Type help for help. mixedDB=# That connection should've matched that 2nd line, and be rejected. Actually it should have matched neither, as both lines will get folded downcase: local mixeddb all trust local mixeddb all reject Christoph -- c...@df7cb.de | http://www.df7cb.de/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] run xmllint during build (was Re: need xmllint on borka)
Hello Peter,
Agreed. I have committed the SGML changes that make things valid now,
but I will postpone the xmllint addition until the 9.5 branch, complete
with more documentation.
Per the above announcement, here is an updated patch, also with more
documentation and explanations.
It would especially be valuable if someone with a different-than-mine OS
would verify whether they can install xmllint according to the
documentation, or update the documentation if not.
Tested on Ubuntu trusty.
Patched applies with some minor warning on current head, and works, that
is xmllint is run for some of the targets (epub, man).
However, it seems that it is not run for target html, nor for pdf/ps
targets. I'm wondering whether it is an oversight or if there is a reason
for that. Maybe the intention is that an explicit htmlhelp is expected
to do the checking, but then why force it for man and epub? It seems to me
that it is not consistent.
Also, a general comment, which is independent of this patch: I found the
documentation build especially not resilient, with a lack of clear error
messages when something is broken. Basically, if configure does not found
something for the doc (openjade, osx, xmllint, ...) it does not complain.
That is fine with me, people would not always want to build the doc anyway
as it is available online. However, the Makefile in doc/src/sgml overrides
the not found commands (ifndef JADE JADE=..., etc), and proceed to
unhelpful and unclear errors later on. ISTM that it may be more helful to
do:
ifndef JADE
#error no jade found on your system, cannot generate the documention
endif
Rather than overriding with JADE=jade if jade was not there when
configuring.
This xmllint addition is done in the same spirit. I would suggest at the
minimum to check that xmllint is available before running it, or to ignore
the call if not available, something like:
type -p $(XMLLINT) || { echo error no $(XMLLINT)... ; exit 1 ; }
$(XMLLINT) ...
or
-type -p $(XMLLINT) $(XMLLINT) ...
And I would prefer that a straightforward error is generated when
commands or styles are not found, in general.
Also, a detail, the Makefile style is not homogeneous:
ifndef XSLTPROC
XSLTPROC = xsltproc
endif
DBTOEPUB ?= dbtoepub
Why not XSLTPROC ?= xsltproc and the like?
--
Fabien.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_receivexlog --status-interval add fsync feedback
On Thu, Aug 21, 2014 at 2:54 PM, furu...@pm.nttdata.co.jp wrote: When replication slot is not specified in pg_receivexlog, the flush location in the feedback message always indicates invalid. So there seems to be no need to send the feedback as soon as fsync is issued, in that case. How should this option work when replication slot is not specified? Thanks for the review! The present is not checking the existence of specification of -S. The use case when replication slot is not specified. Because it does fsync, it isn't an original intention. remote_write is set in synchronous_commit. To call attention to the user, append following documents. If you want to report the flush position to the server, should use -S option. Thank you for updating the patch. I reviewed the patch. First of all, I think that we should not append the above message to section of '-r' option. (Or these message might not be needed at all) Whether flush location in feedback message is valid, is not depend on '-r' option. If we use '-r' option and 'S' option (i.g., replication slot) then pg_receivexlog informs valid flush location to primary server at the same time as doing fsync. But, if we don't specify replication slot then the flush location in feedback message always invalid. So I think Fujii-san pointed out that sending of invalid flush location is not needed if pg_receivexlog does not use replication slot. 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: Compute attr_needed for child relations (was Re: [HACKERS] inherit support for foreign tables)
(2014/08/21 13:21), Ashutosh Bapat wrote: On Wed, Aug 20, 2014 at 3:25 PM, Etsuro Fujita fujita.ets...@lab.ntt.co.jp mailto:fujita.ets...@lab.ntt.co.jp wrote: Hi Ashutish, I am sorry that I mistook your name's spelling. (2014/08/14 22:30), Ashutosh Bapat wrote: On Thu, Aug 14, 2014 at 10:05 AM, Etsuro Fujita fujita.ets...@lab.ntt.co.jp mailto:fujita.ets...@lab.ntt.co.jp mailto:fujita.ets...@lab.ntt.__co.jp mailto:fujita.ets...@lab.ntt.co.jp wrote: (2014/08/08 18:51), Etsuro Fujita wrote: (2014/06/30 22:48), Tom Lane wrote: I wonder whether it isn't time to change that. It was coded like that originally only because calculating the values would've been a waste of cycles at the time. But this is at least the third place where it'd be useful to have attr_needed for child rels. There was a problem with the previous patch, which will be described below. Attached is the updated version of the patch addressing that. Here are some more comments attr_needed also has the attributes used in the restriction clauses as seen in distribute_qual_to_rels(), so, it looks unnecessary to call pull_varattnos() on the clauses in baserestrictinfo in functions check_selective_binary___conversion(), remove_unused_subquery___outputs(), check_index_only(). IIUC, I think it's *necessary* to do that in those functions since the attributes used in the restriction clauses in baserestrictinfo are not added to attr_needed due the following code in distribute_qual_to_rels. That's right. Thanks for pointing that out. Although in case of RTE_RELATION, the 0th entry in attr_needed corresponds to FirstLowInvalidHeapAttributeNu__mber + 1, it's always safer to use it is RelOptInfo::min_attr, in case get_relation_info() wants to change assumption or somewhere down the line some other part of code wants to change attr_needed information. It may be unlikely, that it would change, but it will be better to stick to comments in RelOptInfo 443 AttrNumber min_attr; /* smallest attrno of rel (often 0) */ 444 AttrNumber max_attr; /* largest attrno of rel */ 445 Relids *attr_needed;/* array indexed [min_attr .. max_attr] */ Good point! Attached is the revised version of the patch. If the patch is not in the commit-fest, can you please add it there? I've already done that: https://commitfest.postgresql.org/action/patch_view?id=1529 From my side, the review is done, it should be marked ready for committer, unless somebody else wants to review. Many thanks! Best regards, Etsuro Fujita -- 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] run xmllint during build (was Re: need xmllint on borka)
Also, a general comment, which is independent of this patch: I found the documentation build especially not resilient, with a lack of clear error messages when something is broken. Basically, if configure does not found something for the doc (openjade, osx, xmllint, ...) it does not complain. That is fine with me, people would not always want to build the doc anyway as it is available online. However, the Makefile in doc/src/sgml overrides the not found commands (ifndef JADE JADE=..., etc), and proceed to unhelpful and unclear errors later on. ISTM that it may be more helful to do: To be more constructive: Maybe all commands could have a check counterpart added to the dependencies, so as to fail gracefully only if needed, something like: .check_XXX: if type -p $(XXX) /dev/null ; then touch $@ ; else \ echo command $(XXX) not found; exit 1 ; \ fi foo: .check_XXX $(XXX) ... I'm not sure how to check for the docbook style availability though. -- Fabien. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] inherit support for foreign tables
(2014/07/02 11:23), Noah Misch wrote: On Fri, Jun 20, 2014 at 05:04:06PM +0900, Kyotaro HORIGUCHI wrote: Attached is the rebased patch of v11 up to the current master. The rest of these review comments are strictly observations; they're not requests for you to change the patch, but they may deserve more discussion. We use the term child table in many messages. Should that change to something more inclusive, now that the child may be a foreign table? Perhaps one of child relation, plain child, or child foreign table/child table depending on the actual object? child relation is robust technically, but it might add more confusion than it removes. Varying the message depending on the actual object feels like a waste. Opinions? IMHO, I think that child table would not confusing users terribly. LOCK TABLE on the inheritance parent locks child foreign tables, but LOCK TABLE fails when given a foreign table directly. That's odd, but I see no cause to change it. I agree wth that. The longstanding behavior of CREATE TABLE INHERITS is to reorder local columns to match the order found in parents. That is, both of these tables actually have columns in the order (a,b): create table parent (a int, b int); create table child (b int, a int) inherits (parent); Ordinary dump/reload always uses CREATE TABLE INHERITS, thereby changing column order in this way. (pg_dump --binary-upgrade uses ALTER TABLE INHERIT and some catalog hacks to avoid doing so.) I've never liked that dump/reload can change column order, but it's tolerable if you follow the best practice of always writing out column lists. The stakes rise for foreign tables, because column order is inherently significant to file_fdw and probably to certain other non-RDBMS FDWs. If pg_dump changes column order in your file_fdw foreign table, the table breaks. I would heartily support making pg_dump preserve column order for all inheritance children. That doesn't rise to the level of being a blocker for this patch, though. I agree with that, too. I think it would be better to add docs for now. Thanks, Best regards, Etsuro Fujita -- 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] documentation update for doc/src/sgml/func.sgml
attached is a small patch which updates doc/src/sgml/func.sgml. The change explains that functions like round() and others might behave different depending on your operating system (because of rint(3)) and that this is according to an IEEE standard. It also points out that #.5 is not always rounded up, as expected from a mathematical point of view. Applied on head read. I'm not a native English speaker, but the English looked right to me. Comments: I'm not sure that the note that on the third line is useful. I do not understand why the last sentence in the first paragraph about bitwise ops is put there with rounding issues, which seem unrelated. It seems to me that it belongs to the second paragraph which is about bitwise operators. The wikipedia link can be simplified to a much cleaner: http://en.wikipedia.org/wiki/IEEE_floating_point#Rounding_rules Also, I submitted docs with relevant wikipedia links which was stripped of these before committing. I'm wondering whether there is a general policy not to put external links from within the text in the documentation. There are very few of them, most in acronym.sgml. I would suggest to put relevant tags around functions and types, like: functionround()/ and typeNUMERIC/. -- Fabien. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] option -T in pg_basebackup doesn't work on windows
On Tue, Aug 19, 2014 at 9:51 AM, Amit Kapila amit.kapil...@gmail.com wrote: On Mon, Aug 18, 2014 at 7:50 PM, Heikki Linnakangas hlinnakan...@vmware.com wrote: Wouldn't it make a lot more sense to create it correctly in the first place? Looking at the code, I think it is very well possible to create it correctly in the first place without much extra work. I will send a patch if nobody sees any problem with this change. Attached patch implements the above suggested fix. I have removed the earlier code which was used to update the symlink path. Do you see any need to change below line in docs: If a tablespace is relocated in this way, the symbolic links inside the main data directory are updated to point to the new location. Refer link: http://www.postgresql.org/docs/devel/static/app-pgbasebackup.html I was not sure whether docs need any change, so kept them intact. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com pg_basebackup_relocate_tablespace_v3.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpgsql.extra_warnings='num_into_expressions'
On 08/07/2014 01:11 AM, Marko Tiikkaja wrote: On 7/21/14, 10:56 PM, I wrote: Here's a patch which allows you to notice those annoying bugs with INTO slightly more quickly. Adding to the next commit phest. New version, fixed bugs with set operations and VALUES lists. Looks good. It seems weird to pass a PLpgSQL_row struct to check_sql_expr. check_sql_expr only needs to know how many attributes is expected to be in the target list, so it would be more natural to just pass an int expected_natts. Once you do that, you could trivially also add checking for other cases, e.g: do $$ declare i int4; begin -- fails at runtime, because SELECT 1,3 returns two attributes, -- but FOR expects 1 for i in 1,3..(2) loop raise notice 'foo %', i; end loop; end; $$; There's probably more checking like that that you could add, but that can be done as add-on patches, if ever. The INTO mistake happens a lot more easily. - 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] documentation update for doc/src/sgml/func.sgml
On 08/21/2014 11:53 AM, Fabien COELHO wrote: attached is a small patch which updates doc/src/sgml/func.sgml. The change explains that functions like round() and others might behave different depending on your operating system (because of rint(3)) and that this is according to an IEEE standard. It also points out that #.5 is not always rounded up, as expected from a mathematical point of view. Applied on head read. I'm not a native English speaker, but the English looked right to me. Thanks. Comments: I'm not sure that the note that on the third line is useful. I do not understand why the last sentence in the first paragraph about bitwise ops is put there with rounding issues, which seem unrelated. It seems to me that it belongs to the second paragraph which is about bitwise operators. That's the part which came from Josh Berkus. We discussed this patch on IRC. The wikipedia link can be simplified to a much cleaner: http://en.wikipedia.org/wiki/IEEE_floating_point#Rounding_rules It can, but then you always refer to the latest version of the Wikipedia page, which might or might not be a good idea. The link in the patch points to the current version from yesterday, no matter how many changes are introduced afterwards. But yes: Also, I submitted docs with relevant wikipedia links which was stripped of these before committing. I'm wondering whether there is a general policy not to put external links from within the text in the documentation. There are very few of them, most in acronym.sgml. It would be nice to have a general rule how to handle external links. I would suggest to put relevant tags around functions and types, like: functionround()/ and typeNUMERIC/. Can do. Thanks, -- Andreas 'ads' Scherbaum German PostgreSQL User Group European PostgreSQL User Group - Board of Directors Volunteer Regional Contact, Germany - PostgreSQL Project -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP Patch for GROUPING SETS phase 1
A progress update: Atri We envisage that handling of arbitrary grouping sets will be Atri best done by having the planner generating an Append of Atri multiple aggregation paths, presumably with some way of moving Atri the original input path to a CTE. We have not really explored Atri yet how hard this will be; suggestions are welcome. This idea was abandoned. Instead, we have implemented full support for arbitrary grouping sets by means of a chaining system: explain (verbose, costs off) select four, ten, hundred, count(*) from onek group by cube(four,ten,hundred); QUERY PLAN - GroupAggregate Output: four, ten, hundred, count(*) Grouping Sets: (onek.hundred, onek.four, onek.ten), (onek.hundred, onek.four), (onek.hundred), () - Sort Output: four, ten, hundred Sort Key: onek.hundred, onek.four, onek.ten - ChainAggregate Output: four, ten, hundred Grouping Sets: (onek.ten, onek.hundred), (onek.ten) - Sort Output: four, ten, hundred Sort Key: onek.ten, onek.hundred - ChainAggregate Output: four, ten, hundred Grouping Sets: (onek.four, onek.ten), (onek.four) - Sort Output: four, ten, hundred Sort Key: onek.four, onek.ten - Seq Scan on public.onek Output: four, ten, hundred (20 rows) The ChainAggregate nodes use a tuplestore to communicate with the GroupAggregate node at the top of the chain; they pass through input tuples unchanged, and write aggregated result rows to the tuplestore, which the top node then returns once it has finished its own result. The organization of the planner code seems to be actively hostile to any attempt to break out new CTEs on the fly, or to plan parts of the query more than once; the method above seems to be the easiest way to avoid those issues. Atri At this point we are more interested in design review rather Atri than necessarily committing this patch in its current state. This no longer applies; we expect to post within a day or two an updated patch with full functionality. -- Andrew (irc:RhodiumToad) -- 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] documentation update for doc/src/sgml/func.sgml
I do not understand why the last sentence in the first paragraph about bitwise ops is put there with rounding issues, which seem unrelated. It seems to me that it belongs to the second paragraph which is about bitwise operators. That's the part which came from Josh Berkus. We discussed this patch on IRC. Hmmm. I do think the last sentence belongs to the next paragraph. The identity of the author does not change my opinion on that point:-) If you have another argument, maybe. The wikipedia link can be simplified to a much cleaner: http://en.wikipedia.org/wiki/IEEE_floating_point#Rounding_rules It can, but then you always refer to the latest version of the Wikipedia page, which might or might not be a good idea. The link in the patch points to the current version from yesterday, no matter how many changes are introduced afterwards. I doubt that IEEE floating point rounding rules are likely to change much, so referencing the latest version is both safe cleaner. Also, wikipedia would change its implementation from php to something else (well, unlikely, probably as unlikely as a change in IEEE fp rounding rules:-). -- Fabien. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP Patch for GROUPING SETS phase 1
On 08/21/2014 01:28 PM, Andrew Gierth wrote: A progress update: Atri We envisage that handling of arbitrary grouping sets will be Atri best done by having the planner generating an Append of Atri multiple aggregation paths, presumably with some way of moving Atri the original input path to a CTE. We have not really explored Atri yet how hard this will be; suggestions are welcome. This idea was abandoned. Instead, we have implemented full support for arbitrary grouping sets by means of a chaining system: explain (verbose, costs off) select four, ten, hundred, count(*) from onek group by cube(four,ten,hundred); QUERY PLAN - GroupAggregate Output: four, ten, hundred, count(*) Grouping Sets: (onek.hundred, onek.four, onek.ten), (onek.hundred, onek.four), (onek.hundred), () - Sort Output: four, ten, hundred Sort Key: onek.hundred, onek.four, onek.ten - ChainAggregate Output: four, ten, hundred Grouping Sets: (onek.ten, onek.hundred), (onek.ten) - Sort Output: four, ten, hundred Sort Key: onek.ten, onek.hundred - ChainAggregate Output: four, ten, hundred Grouping Sets: (onek.four, onek.ten), (onek.four) - Sort Output: four, ten, hundred Sort Key: onek.four, onek.ten - Seq Scan on public.onek Output: four, ten, hundred (20 rows) Uh, that's ugly. The EXPLAIN out I mean; as an implementation detail chaining the nodes might be reasonable. But the above gets unreadable if you have more than a few grouping sets. The ChainAggregate nodes use a tuplestore to communicate with the GroupAggregate node at the top of the chain; they pass through input tuples unchanged, and write aggregated result rows to the tuplestore, which the top node then returns once it has finished its own result. Hmm, so there's a magic link between the GroupAggregate at the top and all the ChainAggregates, via the tuplestore. That may be fine, we have special rules in passing information between bitmap scan nodes too. But rather than chain multiple ChainAggregate nodes, how about just doing all the work in the top GroupAggregate node? Atri At this point we are more interested in design review rather Atri than necessarily committing this patch in its current state. This no longer applies; we expect to post within a day or two an updated patch with full functionality. Ok, cool - 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] plpgsql.extra_warnings='num_into_expressions'
On 8/21/14, 1:19 PM, Heikki Linnakangas wrote: On 08/07/2014 01:11 AM, Marko Tiikkaja wrote: On 7/21/14, 10:56 PM, I wrote: Here's a patch which allows you to notice those annoying bugs with INTO slightly more quickly. Adding to the next commit phest. New version, fixed bugs with set operations and VALUES lists. Looks good. There's probably more checking like that that you could add, but that can be done as add-on patches, if ever. The INTO mistake happens a lot more easily. Yeah, I think the mistake is at least as easy to do in FOR .. IN query, and I'm planning to add checks for that as well. But I haven't found the time to look at it amongst all the other patches and projects I have going (and also, unfortunately, I'm on vacation right now). It seems weird to pass a PLpgSQL_row struct to check_sql_expr. check_sql_expr only needs to know how many attributes is expected to be in the target list, so it would be more natural to just pass an int expected_natts. I'm not sure about this, though. AFAICT all the interesting cases are already holding a PLpgSQL_row, and in that case it seems easier to just pass that in to check_sql_expr() without making the callers worry about extracting the expected_natts from the row. And we can always change the interface should such a case come up, since the interface is completely internal. Just my 0.02EUR, of course. .marko -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpgsql.extra_warnings='num_into_expressions'
On 08/21/2014 02:09 PM, Marko Tiikkaja wrote: On 8/21/14, 1:19 PM, Heikki Linnakangas wrote: On 08/07/2014 01:11 AM, Marko Tiikkaja wrote: On 7/21/14, 10:56 PM, I wrote: Here's a patch which allows you to notice those annoying bugs with INTO slightly more quickly. Adding to the next commit phest. New version, fixed bugs with set operations and VALUES lists. Looks good. There's probably more checking like that that you could add, but that can be done as add-on patches, if ever. The INTO mistake happens a lot more easily. Yeah, I think the mistake is at least as easy to do in FOR .. IN query, and I'm planning to add checks for that as well. But I haven't found the time to look at it amongst all the other patches and projects I have going Ok. (and also, unfortunately, I'm on vacation right now). Oh, have fun! It seems weird to pass a PLpgSQL_row struct to check_sql_expr. check_sql_expr only needs to know how many attributes is expected to be in the target list, so it would be more natural to just pass an int expected_natts. I'm not sure about this, though. AFAICT all the interesting cases are already holding a PLpgSQL_row, and in that case it seems easier to just pass that in to check_sql_expr() without making the callers worry about extracting the expected_natts from the row. Hmm. The integer FOR syntax I used in my example does not, it always expects 1 output column. And we can always change the interface should such a case come up, since the interface is completely internal. Just my 0.02EUR, of course. You might want to add a helper function to count the number of attributes in a PLpgSQL_row. Then the check_sql_expr call would be almost as simple: check_sql_expr(..., get_row_natts(row)). - 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] proposal: rounding up time value less than its unit.
On 07/10/2014 09:52 AM, Tomonari Katsumata wrote: Hi, Several couple weeks ago, I posted a mail to pgsql-doc. http://www.postgresql.org/message-id/53992ff8.2060...@po.ntts.co.jp In this thread, I concluded that it's better to round up the value which is less than its unit. Current behavior (rounding down) has undesirable setting risk, because some GUCs have special meaning for 0. And then I made a patch for this. Please check the attached patch. The patch also rounds a zero up to one. A naked zero with no unit is not affected, but e.g if you have log_rotation_age=0s, it will not disable the feature as you might expect, but set it to 1 minute. Should we do something about that? If we're going to explain the rounding up in the manual, we also need to explain the normal rule, which is to round down. How about this: --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -44,6 +44,15 @@ (seconds), literalmin/literal (minutes), literalh/literal (hours), and literald/literal (days). Note that the multiplier for memory units is 1024, not 1000. + +para + If a memory or time setting is specified with more precision than the + implicit unit of the setting, it is rounded down. However, if rounding + down would yield a zero, it is rounded up to one instead. For example, + the implicit unit of varnamelog_rotation_age/varname is minutes, so if + you set it to literal150s/literal, it will be rounded down to two + minutes. However, if you set it to literal10s/literal, it will be + rounded up to one minute. /para - 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] Removing dependency to wsock32.lib when compiling code on WIndows
On 08/15/2014 11:00 PM, Noah Misch wrote: On Fri, Aug 15, 2014 at 12:49:36AM -0700, Michael Paquier wrote: Btw, how do you determine if MSVC is using HAVE_GETADDRINFO? Is it decided by the inclusion of getaddrinfo.c in @pgportfiles of Mkvdbuild.pm? src/include/pg_config.h.win32 dictates it, and we must keep @pgportfiles synchronized with the former's verdict. What's happening about this? Buildfarm animal jacana is consistently red because of this. 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] [GSoC2014] Patch ALTER TABLE ... SET LOGGED
On Thu, Aug 21, 2014 at 5:23 AM, Christoph Berg c...@df7cb.de wrote:
Re: Thom Brown 2014-08-20 CAA-aLv7TeF8iM=
7u7tsgl4s5jh1a+shq_ny7gorzc_g_yj7...@mail.gmail.com
ERROR: table test is not permanent
Perhaps this would be better as table test is unlogged as permanent
doesn't match the term used in the DDL syntax.
I was also wondering that, but then figured that when ALTER TABLE SET
UNLOGGED is invoked on temp tables, the error message is not
permanent was correct while the apparent opposite is unlogged is
wrong.
Christoph
--
c...@df7cb.de | http://www.df7cb.de/
Thom,
Christoph is right... make no sense the message... see the example:
fabrizio=# create temp table foo();
CREATE TABLE
fabrizio=# alter table foo set unlogged;
ERROR: table foo is unlogged
The previous message is better:
fabrizio=# create temp table foo();
CREATE TABLE
fabrizio=# alter table foo set unlogged;
ERROR: table foo is not permanent
fabrizio=#
fabrizio=# create unlogged table foo2();
CREATE TABLE
fabrizio=# alter table foo2 set unlogged;
ERROR: table foo2 is not permanent
Patch attached.
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
diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index 69a1e14..397b4e6 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -59,16 +59,17 @@ ALTER TABLE [ IF EXISTS ] replaceable class=PARAMETERname/replaceable
ENABLE ALWAYS RULE replaceable class=PARAMETERrewrite_rule_name/replaceable
CLUSTER ON replaceable class=PARAMETERindex_name/replaceable
SET WITHOUT CLUSTER
+SET {LOGGED | UNLOGGED}
SET WITH OIDS
SET WITHOUT OIDS
SET ( replaceable class=PARAMETERstorage_parameter/replaceable = replaceable class=PARAMETERvalue/replaceable [, ... ] )
+SET TABLESPACE replaceable class=PARAMETERnew_tablespace/replaceable
RESET ( replaceable class=PARAMETERstorage_parameter/replaceable [, ... ] )
INHERIT replaceable class=PARAMETERparent_table/replaceable
NO INHERIT replaceable class=PARAMETERparent_table/replaceable
OF replaceable class=PARAMETERtype_name/replaceable
NOT OF
OWNER TO replaceable class=PARAMETERnew_owner/replaceable
-SET TABLESPACE replaceable class=PARAMETERnew_tablespace/replaceable
REPLICA IDENTITY {DEFAULT | USING INDEX replaceable class=PARAMETERindex_name/replaceable | FULL | NOTHING}
phraseand replaceable class=PARAMETERtable_constraint_using_index/replaceable is:/phrase
@@ -447,6 +448,20 @@ ALTER TABLE [ IF EXISTS ] replaceable class=PARAMETERname/replaceable
/varlistentry
varlistentry
+termliteralSET {LOGGED | UNLOGGED}/literal/term
+listitem
+ para
+ This form changes the table persistence type from unlogged to permanent or
+ from permanent to unlogged (see xref linkend=SQL-CREATETABLE-UNLOGGED).
+ /para
+ para
+ Changing the table persistence type acquires an literalACCESS EXCLUSIVE/literal lock
+ and rewrites the table contents and associated indexes into new disk files.
+ /para
+/listitem
+ /varlistentry
+
+ varlistentry
termliteralSET WITH OIDS/literal/term
listitem
para
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index b1c411a..7f497c7 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -574,7 +574,8 @@ rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose)
heap_close(OldHeap, NoLock);
/* Create the transient table that will receive the re-ordered data */
- OIDNewHeap = make_new_heap(tableOid, tableSpace, false,
+ OIDNewHeap = make_new_heap(tableOid, tableSpace,
+ OldHeap-rd_rel-relpersistence,
AccessExclusiveLock);
/* Copy the heap data into the new table in the desired order */
@@ -601,7 +602,7 @@ rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose)
* data, then call finish_heap_swap to complete the operation.
*/
Oid
-make_new_heap(Oid OIDOldHeap, Oid NewTableSpace, bool forcetemp,
+make_new_heap(Oid OIDOldHeap, Oid NewTableSpace, char relpersistence,
LOCKMODE lockmode)
{
TupleDesc OldHeapDesc;
@@ -613,7 +614,6 @@ make_new_heap(Oid OIDOldHeap, Oid NewTableSpace, bool forcetemp,
Datum reloptions;
bool isNull;
Oid namespaceid;
- char relpersistence;
OldHeap = heap_open(OIDOldHeap, lockmode);
OldHeapDesc = RelationGetDescr(OldHeap);
@@ -636,16 +636,10 @@ make_new_heap(Oid OIDOldHeap, Oid NewTableSpace, bool forcetemp,
if (isNull)
reloptions = (Datum) 0;
- if (forcetemp)
- {
+ if (relpersistence == RELPERSISTENCE_TEMP)
namespaceid = LookupCreationNamespace(pg_temp);
- relpersistence = RELPERSISTENCE_TEMP;
- }
else
- {
namespaceid =
Re: [HACKERS] [GSoC2014] Patch ALTER TABLE ... SET LOGGED
On 21 August 2014 14:45, Fabrízio de Royes Mello fabriziome...@gmail.com wrote: On Thu, Aug 21, 2014 at 5:23 AM, Christoph Berg c...@df7cb.de wrote: Re: Thom Brown 2014-08-20 CAA-aLv7TeF8iM= 7u7tsgl4s5jh1a+shq_ny7gorzc_g_yj7...@mail.gmail.com ERROR: table test is not permanent Perhaps this would be better as table test is unlogged as permanent doesn't match the term used in the DDL syntax. I was also wondering that, but then figured that when ALTER TABLE SET UNLOGGED is invoked on temp tables, the error message is not permanent was correct while the apparent opposite is unlogged is wrong. Christoph -- c...@df7cb.de | http://www.df7cb.de/ Thom, Christoph is right... make no sense the message... see the example: fabrizio=# create temp table foo(); CREATE TABLE fabrizio=# alter table foo set unlogged; ERROR: table foo is unlogged The previous message is better: fabrizio=# create temp table foo(); CREATE TABLE fabrizio=# alter table foo set unlogged; ERROR: table foo is not permanent fabrizio=# fabrizio=# create unlogged table foo2(); CREATE TABLE fabrizio=# alter table foo2 set unlogged; ERROR: table foo2 is not permanent To me, that's even more confusing: CREATE TEMP TABLE test(); CREATE UNLOGGED TABLE test2(); # ALTER TABLE test SET LOGGED; ERROR: table test is not unlogged # ALTER TABLE test SET UNLOGGED; ERROR: table test is not permanent # ALTER TABLE test2 SET UNLOGGED; ERROR: table test2 is not permanent They're being rejected for different reasons but the error message is identical. Permanent suggests the opposite of temporary, and unlogged tables aren't temporary. -- Thom
Re: [HACKERS] add line number as prompt option to psql
Jeevan Chalke wrote: I have initialize cur_lineno to UINTMAX - 2. And then observed following behaviour to check wrap-around. postgres=# \set PROMPT1 '%/[%l]%R%# ' postgres[18446744073709551613]=# \set PROMPT2 '%/[%l]%R%# ' postgres[18446744073709551613]=# select postgres[18446744073709551614]-# a postgres[18446744073709551615]-# , postgres[0]-# b postgres[1]-# from postgres[2]-# dual; It is wrapping to 0, where as line number always start with 1. Any issues? I would like to ignore this as UINTMAX lines are too much for a input buffer to hold. It is almost NIL chances to hit this. Yeah, most likely you will run out of memory before reaching that point, or out of patience. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP Patch for GROUPING SETS phase 1
Andrew Gierth and...@tao11.riddles.org.uk writes: Heikki == Heikki Linnakangas hlinnakan...@vmware.com writes: Heikki I think we should bite the bullet and rename the extension, I agree, the contrib/cube patch as posted is purely so we could test everything without having to argue over the new name first. I wonder if you've tried hard enough to avoid reserving the keyword. I think that the cube extension is not going to be the only casualty if cube becomes a reserved word --- that seems like a name that could be in use in lots of applications. (What do you mean, 9.5 breaks our database for tracking office space?) It would be worth quite a bit of effort to avoid 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] WIP Patch for GROUPING SETS phase 1
Heikki == Heikki Linnakangas hlinnakan...@vmware.com writes: Heikki Uh, that's ugly. The EXPLAIN out I mean; as an implementation Heikki detail chaining the nodes might be reasonable. But the above Heikki gets unreadable if you have more than a few grouping sets. It's good for highlighting performance issues in EXPLAIN, too. 4096 grouping sets takes about a third of a second to plan and execute, but something like a minute to generate the EXPLAIN output. However, for more realistic sizes, plan time is not significant and explain takes only about 40ms for 256 grouping sets. (To avoid resource exhaustion issues, we have set a limit of, currently, 4096 grouping sets per query level. Without such a limit, it is easy to write queries that would take TBs of memory to parse or plan. MSSQL and DB2 have similar limits, I'm told.) The ChainAggregate nodes use a tuplestore to communicate with the GroupAggregate node at the top of the chain; they pass through input tuples unchanged, and write aggregated result rows to the tuplestore, which the top node then returns once it has finished its own result. Heikki Hmm, so there's a magic link between the GroupAggregate at Heikki the top and all the ChainAggregates, via the tuplestore. That Heikki may be fine, we have special rules in passing information Heikki between bitmap scan nodes too. Eh. It's far from a perfect solution, but the planner doesn't lend itself to perfect solutions. Heikki But rather than chain multiple ChainAggregate nodes, how Heikki about just doing all the work in the top GroupAggregate node? It was easier this way. (How would you expect to do it all in the top node when each subset of the grouping sets list needs to see the data in a different order?) -- Andrew (irc:RhodiumToad) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP Patch for GROUPING SETS phase 1
2014-08-21 17:00 GMT+02:00 Tom Lane t...@sss.pgh.pa.us: Andrew Gierth and...@tao11.riddles.org.uk writes: Heikki == Heikki Linnakangas hlinnakan...@vmware.com writes: Heikki I think we should bite the bullet and rename the extension, I agree, the contrib/cube patch as posted is purely so we could test everything without having to argue over the new name first. I wonder if you've tried hard enough to avoid reserving the keyword. I think that the cube extension is not going to be the only casualty if cube becomes a reserved word --- that seems like a name that could be in use in lots of applications. (What do you mean, 9.5 breaks our database for tracking office space?) It would be worth quite a bit of effort to avoid that. My prototypes worked without reserved keywords if I remember well but analyzer is relatively complex Pavel 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] Hardening pg_upgrade
Now that everyone is happy with how pg_upgrade_support uses global
variables to set preserved oids (or at least has no better ideas), I
think it is time to lock down this usage to prevent future breakage.
Specifically, the first attached patch causes pg_upgrade_support
functions to throw errors when called by a backend that is not in binary
upgrade mode. (This seems like a good safety measure.) Second, and
more importantly, the patch prevents automatic oid assignment when in
binary upgrade mode, except for temporary objects. This is to help
guarantee that system-assigned oids do not conflict with preserved oids.
I had to make an exception for temporary tables because pg_upgrade uses
temporary tables to collect schema information. I tried writing the
query to use CTEs (second patch), but I would then have to have one
query for 8.3, which doesn't support CTEs, and another for 8.4+, plus
the CTE query was more complex than I liked. Another idea would be to
drop 8.3 support (and remove lots of code to support that), but the
recent large increase in the number of people upgrading from 8.4 makes
that unattractive. (8.3 did use a different timestamp storage format
though.)
(Of course, the assumption is that temporary tables will not exist at
the time you are assigning preserved oids.)
Barring objections, I plan to apply the first patch to head, and discard
the second patch.
FYI, I think the macro isTempOrToastNamespace() is misnamed --- it is
testing for temporary tables or temporary toast tables, not for any
toast table. Seems it should be called isTempOrToastTempNamespace().
Should I rename it in a separate commit?
--
Bruce Momjian br...@momjian.ushttp://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
diff --git a/contrib/pg_upgrade_support/pg_upgrade_support.c b/contrib/pg_upgrade_support/pg_upgrade_support.c
new file mode 100644
index edd41d0..beaee76
*** a/contrib/pg_upgrade_support/pg_upgrade_support.c
--- b/contrib/pg_upgrade_support/pg_upgrade_support.c
*** PG_FUNCTION_INFO_V1(set_next_pg_authid_o
*** 38,49
--- 38,57
PG_FUNCTION_INFO_V1(create_empty_extension);
+ #define CHECK_IS_BINARY_UPGRADE \
+ do { \
+ if (!IsBinaryUpgrade) \
+ ereport(ERROR, \
+ (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM), \
+ (errmsg(function can only be called when server is in binary upgrade mode; \
+ } while (0)
Datum
set_next_pg_type_oid(PG_FUNCTION_ARGS)
{
Oid typoid = PG_GETARG_OID(0);
+ CHECK_IS_BINARY_UPGRADE;
binary_upgrade_next_pg_type_oid = typoid;
PG_RETURN_VOID();
*** set_next_array_pg_type_oid(PG_FUNCTION_A
*** 54,59
--- 62,68
{
Oid typoid = PG_GETARG_OID(0);
+ CHECK_IS_BINARY_UPGRADE;
binary_upgrade_next_array_pg_type_oid = typoid;
PG_RETURN_VOID();
*** set_next_toast_pg_type_oid(PG_FUNCTION_A
*** 64,69
--- 73,79
{
Oid typoid = PG_GETARG_OID(0);
+ CHECK_IS_BINARY_UPGRADE;
binary_upgrade_next_toast_pg_type_oid = typoid;
PG_RETURN_VOID();
*** set_next_heap_pg_class_oid(PG_FUNCTION_A
*** 74,79
--- 84,90
{
Oid reloid = PG_GETARG_OID(0);
+ CHECK_IS_BINARY_UPGRADE;
binary_upgrade_next_heap_pg_class_oid = reloid;
PG_RETURN_VOID();
*** set_next_index_pg_class_oid(PG_FUNCTION_
*** 84,89
--- 95,101
{
Oid reloid = PG_GETARG_OID(0);
+ CHECK_IS_BINARY_UPGRADE;
binary_upgrade_next_index_pg_class_oid = reloid;
PG_RETURN_VOID();
*** set_next_toast_pg_class_oid(PG_FUNCTION_
*** 94,99
--- 106,112
{
Oid reloid = PG_GETARG_OID(0);
+ CHECK_IS_BINARY_UPGRADE;
binary_upgrade_next_toast_pg_class_oid = reloid;
PG_RETURN_VOID();
*** set_next_pg_enum_oid(PG_FUNCTION_ARGS)
*** 104,109
--- 117,123
{
Oid enumoid = PG_GETARG_OID(0);
+ CHECK_IS_BINARY_UPGRADE;
binary_upgrade_next_pg_enum_oid = enumoid;
PG_RETURN_VOID();
*** set_next_pg_authid_oid(PG_FUNCTION_ARGS)
*** 114,119
--- 128,134
{
Oid authoid = PG_GETARG_OID(0);
+ CHECK_IS_BINARY_UPGRADE;
binary_upgrade_next_pg_authid_oid = authoid;
PG_RETURN_VOID();
}
*** create_empty_extension(PG_FUNCTION_ARGS)
*** 129,134
--- 144,151
Datum extCondition;
List *requiredExtensions;
+ CHECK_IS_BINARY_UPGRADE;
+
if (PG_ARGISNULL(4))
extConfig = PointerGetDatum(NULL);
else
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
new file mode 100644
index 33eef9f..d810fe9
*** a/src/backend/catalog/heap.c
--- b/src/backend/catalog/heap.c
***
*** 39,44
--- 39,45
#include catalog/dependency.h
#include catalog/heap.h
#include catalog/index.h
+ #include catalog/namespace.h
#include catalog/objectaccess.h
#include catalog/pg_attrdef.h
Re: [HACKERS] proposal: rounding up time value less than its unit.
Heikki Linnakangas hlinnakan...@vmware.com writes: The patch also rounds a zero up to one. A naked zero with no unit is not affected, but e.g if you have log_rotation_age=0s, it will not disable the feature as you might expect, but set it to 1 minute. Should we do something about that? That sounds like a dealbreaker to me. There are enough places where zero has special meaning that we should not *ever* change zero to non-zero silently. 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] Proposal to add a QNX 6.5 port to PostgreSQL
Hello Andres,
Thanks for your response.
About SA_RESTART:
I would like to offer you a different perspective which may alter your current
opinion.
I believe the port.h QNX macro replacement for SA_RESTART is still a reasonable
solution on QNX for these reasons:
First, I think it is better to adapt PostgreSQL to suit the platform than to
adapt the platform to suit PostgreSQL.
Changing default behavior of libc on QNX to suit PostgreSQL may break other
applications which rely on the current behavior of libc.
Yes, I could forget to add a port.h macro for a given interruptible primitive,
but I could likewise forget to update the wrapper for that call in a custom
libc.
I requested that QNX support provide me a list of interruptible primitives, but
I was able to identify many by searching through the QNX help.
Definition of a new interruptible primitive is a rare event, so once a solid
list of macros is in place for QNX, it should need very little maintenance.
If you have any specific calls you believe are missing from my list of macros,
I would be happy to add them.
port.h is included in c.h, which is in postgres.h, so the QNX macros should be
effective for all QNX PostgreSQL compiles.
If it were not, no one could reply on any port.h features on any platform.
Testing so far has demonstrated that the macro fixes are effective on QNX.
Repeated runs of the regression tests run cleanly.
More testing will be required to boost the confidence and expose any gaps, but
the foundation appears to be solid.
The first release on any platform has risk of defects, which can be corrected
once identified.
I would expect that a first release on any platform would include a warning or
disclaimer stating that it is new port.
Lastly, the QNX-specific section added to port.h appears to solve the
SA_RESTART issue for QNX, while having no impact on compiles of existing
platforms.
About configure:
./configure barked at 2 things on QNX, and it advised using both
--without-readline --disable-thread-safety.
I can investigate further, but I have been focusing on the bigger issues first.
I hope the explanations above address your main concerns.
Again, thanks for your response!
Keith Baker
-Original Message-
From: Andres Freund [mailto:and...@2ndquadrant.com]
Sent: Wednesday, August 20, 2014 7:25 PM
To: Baker, Keith [OCDUS Non-JJ]
Cc: Alvaro Herrera; Robert Haas; Tom Lane; pgsql-hackers@postgresql.org
Subject: Re: [HACKERS] Proposal to add a QNX 6.5 port to PostgreSQL
Hi,
On 2014-08-20 21:21:41 +, Baker, Keith [OCDUS Non-JJ] wrote:
To work around lack of SA_RESTART, I added QNX-specific retry macros
to port.h With these macros in place make check runs cleanly (fails in
many place without them).
+#if defined(__QNX__)
+/* QNX does not support sigaction SA_RESTART. We must retry
interrupted calls (EINTR) */
+/* Helper macros, used to build our retry macros */
+#define PG_RETRY_EINTR3(exp,val,type) ({ type _tmp_rc; do _tmp_rc =
(exp); while (_tmp_rc == (val) errno == EINTR); _tmp_rc; })
+#define PG_RETRY_EINTR(exp) PG_RETRY_EINTR3(exp,-1L,long int)
+#define PG_RETRY_EINTR_FILE(exp) PG_RETRY_EINTR3(exp,NULL,FILE
*)
+/* override calls known to return EINTR when interrupted */
+#define close(a) PG_RETRY_EINTR(close(a))
+#define fclose(a) PG_RETRY_EINTR(fclose(a))
+#define fdopen(a,b) PG_RETRY_EINTR_FILE(fdopen(a,b))
+#define fopen(a,b) PG_RETRY_EINTR_FILE(fopen(a,b))
+#define freopen(a,b,c) PG_RETRY_EINTR_FILE(freopen(a,b,c))
+#define fseek(a,b,c) PG_RETRY_EINTR(fseek(a,b,c))
+#define fseeko(a,b,c) PG_RETRY_EINTR(fseeko(a,b,c))
+#define ftruncate(a,b) PG_RETRY_EINTR(ftruncate(a,b))
+#define lseek(a,b,c) PG_RETRY_EINTR(lseek(a,b,c))
+#define open(a,b,...) ({ int _tmp_rc; do _tmp_rc =
open(a,b,##__VA_ARGS__); while (_tmp_rc == (-1) errno == EINTR);
_tmp_rc; })
+#define shm_open(a,b,c) PG_RETRY_EINTR(shm_open(a,b,c))
+#define stat(a,b) PG_RETRY_EINTR(stat(a,b))
+#define unlink(a) PG_RETRY_EINTR(unlink(a))
... (Macros for read and write are similar but slightly longer, so I
omit
them here)...
+#endif /* __QNX__ */
I think this is a horrible way to go and unlikely to succeed. You're surely
going
to miss calls and it's going to need to be maintained continuously. We'll miss
adding things which will then only break under load. Which most poeple
won't be able to generate under qnx.
The only reasonably way to fake kernel SA_RESTART support is doing so is in
$platform's libc. In the syscall wrapper.
Here is what I used for configure, I am open to suggestions:
./configure --without-readline --disable-thread-safety
Why is the --disable-thread-safety needed?
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL
Re: [HACKERS] WIP Patch for GROUPING SETS phase 1
Tom == Tom Lane t...@sss.pgh.pa.us writes: I agree, the contrib/cube patch as posted is purely so we could test everything without having to argue over the new name first. Tom I wonder if you've tried hard enough to avoid reserving the keyword. GROUP BY cube(a,b) is currently legal syntax and means something completely incompatible to what the spec requires. GROUP BY GROUPING SETS (cube(a,b), c) -- is that cube(a,b) an expression to group on, or a list of grouping sets to expand? GROUP BY (cube(a,b)) -- should that be an error, or silently treat it as a function call rather than a grouping set? What about GROUP BY GROUPING SETS ((cube(a,b)) ? (both are errors in our patch) Accepting those as valid implies a degree of possible confusion that I personally regard as quite questionable. Previous discussion seemed to have accepted that contrib/cube was going to have to be renamed. Tom I think that the cube extension is not going to be the only Tom casualty if cube becomes a reserved word --- that seems like a Tom name that could be in use in lots of applications. (What do Tom you mean, 9.5 breaks our database for tracking office space?) Tom It would be worth quite a bit of effort to avoid that. It has been a reserved word in the spec since, what, 1999? and it is a reserved word in mssql, oracle, db2, etc.? It only needs to be a col_name_keyword, so it still works as a table or column name (as usual we are less strict than the spec in that respect). I'm looking into whether it can be made unreserved, but I have serious doubts about this being a good idea. -- Andrew (irc:RhodiumToad) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP Patch for GROUPING SETS phase 1
2014-08-21 17:58 GMT+02:00 Andrew Gierth and...@tao11.riddles.org.uk: Tom == Tom Lane t...@sss.pgh.pa.us writes: I agree, the contrib/cube patch as posted is purely so we could test everything without having to argue over the new name first. Tom I wonder if you've tried hard enough to avoid reserving the keyword. GROUP BY cube(a,b) is currently legal syntax and means something completely incompatible to what the spec requires. GROUP BY GROUPING SETS (cube(a,b), c) -- is that cube(a,b) an expression to group on, or a list of grouping sets to expand? GROUP BY (cube(a,b)) -- should that be an error, or silently treat it as a function call rather than a grouping set? What about GROUP BY GROUPING SETS ((cube(a,b)) ? (both are errors in our patch) Accepting those as valid implies a degree of possible confusion that I personally regard as quite questionable. Previous discussion seemed to have accepted that contrib/cube was going to have to be renamed. Tom I think that the cube extension is not going to be the only Tom casualty if cube becomes a reserved word --- that seems like a Tom name that could be in use in lots of applications. (What do Tom you mean, 9.5 breaks our database for tracking office space?) Tom It would be worth quite a bit of effort to avoid that. It has been a reserved word in the spec since, what, 1999? and it is a reserved word in mssql, oracle, db2, etc.? It only needs to be a col_name_keyword, so it still works as a table or column name (as usual we are less strict than the spec in that respect). I'm looking into whether it can be made unreserved, but I have serious doubts about this being a good idea. +1 contrib module should be renamed - more - current name is confusing against usual functionality related to words CUBE and ROLLUP Pavel -- Andrew (irc:RhodiumToad) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_dumpall reccomendation in release notes
On Tue, Feb 25, 2014 at 05:05:09PM -0800, Josh Berkus wrote: On 02/25/2014 04:42 PM, Bruce Momjian wrote: On Tue, Feb 25, 2014 at 06:41:26PM -0500, Tom Lane wrote: I'm not sure what many limitations you think pg_dumpall has that pg_dump doesn't. I do think that it might be time to reword this to recommend pg_upgrade first, though. ISTM that the current wording dates from when pg_upgrade could charitably be described as experimental. Wow, so pg_upgrade takes the lead! And from Tom too! :-) I agree with Tom that mentioning pg_dump/restore is going to lead to global object data loss, and throwing the users to a URL with no explanation isn't going to help either. What we could do is to restructure the existing text and add a link to the upgrade URL for more details. What I was suggesting was something like: Users upgrading from earlier versions will need to go through the entire upgrade procedure, as described on our upgrade page: link The problem is that anything we say about how to upgrade in one short sentence is going to confuse some people. BTW, the reason I got that question about pg_dump was that 9.2's release notes say pg_dump and 9.3's say pg_dumpall, causing users to think there's been some kind of change. Of course, this means I need to fix the upgrade page, and I need to write backported versions of that fix for at least 9.1 and 9.2. I have developed the attached patch to address the issues raised above: o non-text output of pg_dump is mentioned o mentions of using OID for keys is removed o the necessity of pg_dumpall --globals-only is mentioned o using pg_dump parallel mode rather than pg_dumpall for upgrades is mentioned o pg_upgrade is mentioned more prominently for upgrades o replication upgrades are in their own section I don't think we want to mention pg_upgrade as the _primary_ major-version upgrade method. While the pg_dump upgrade section is long, it is mostly about starting/stoping the server, moving directories, etc, the same things you have to do for pg_upgrade, so I just mentioned that int the pg_upgrade section. Other ideas? I plan to apply this to head and 9.4. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml new file mode 100644 index 06f064e..07ca0dc *** a/doc/src/sgml/backup.sgml --- b/doc/src/sgml/backup.sgml *** *** 28,34 titleacronymSQL/ Dump/title para !The idea behind this dump method is to generate a text file with SQL commands that, when fed back to the server, will recreate the database in the same state as it was at the time of the dump. productnamePostgreSQL/ provides the utility program --- 28,34 titleacronymSQL/ Dump/title para !The idea behind this dump method is to generate a file with SQL commands that, when fed back to the server, will recreate the database in the same state as it was at the time of the dump. productnamePostgreSQL/ provides the utility program *** pg_dump replaceable class=parameterd *** 39,44 --- 39,47 /synopsis As you see, applicationpg_dump/ writes its result to the standard output. We will see below how this can be useful. +While the above command creates a text file, applicationpg_dump/ +can create files in other formats that allow for parallism and more +fine-grained control of object restoration. /para para *** pg_dump replaceable class=parameterd *** 98,117 exclusive lock, such as most forms of commandALTER TABLE/command.) /para - important -para - If your database schema relies on OIDs (for instance, as foreign - keys) you must instruct applicationpg_dump/ to dump the OIDs - as well. To do this, use the option-o/option command-line - option. -/para - /important - sect2 id=backup-dump-restore titleRestoring the Dump/title para ! The text files created by applicationpg_dump/ are intended to be read in by the applicationpsql/application program. The general command form to restore a dump is synopsis --- 101,111 exclusive lock, such as most forms of commandALTER TABLE/command.) /para sect2 id=backup-dump-restore titleRestoring the Dump/title para ! Text files created by applicationpg_dump/ are intended to be read in by the applicationpsql/application program. The general command form to restore a dump is synopsis *** psql replaceable class=parameterdbna *** 127,132 --- 121,128 supports options similar to applicationpg_dump/ for specifying the database server to connect to and the user name to use. See the xref linkend=app-psql reference page for more information. + Non-text
Re: [HACKERS] Proposal to add a QNX 6.5 port to PostgreSQL
Baker, Keith [OCDUS Non-JJ] wrote: About configure: ./configure barked at 2 things on QNX, and it advised using both --without-readline --disable-thread-safety. I can investigate further, but I have been focusing on the bigger issues first. I don't think thread-safety is of great concern. The backend is not multithreaded, and neither are the utilities (I think the only exception is pgbench, and even there it is optional). The only problem, as I recall, would be that libpq would not lock things correctly when used in a multithreaded program. I think you will need to solve this eventually, but it doesn't look as critical as the others. I was asking specifically about spinlocks because if you have to use that switch, it means our spinlock implementation doesn't cover your platform, and you would need to add something to support native spinlocks. Since you're using gcc on x86, I assume your port is choosing an already existing, working implementation. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP Patch for GROUPING SETS phase 1
Andrew Gierth and...@tao11.riddles.org.uk writes: Tom == Tom Lane t...@sss.pgh.pa.us writes: Tom I wonder if you've tried hard enough to avoid reserving the keyword. GROUP BY cube(a,b) is currently legal syntax and means something completely incompatible to what the spec requires. Well, if there are any extant applications that use that exact phrasing, they're going to be broken in any case. That does not mean that we have to break every other appearance of cube. I think that special-casing appearances of cube(...) in GROUP BY lists might be a feasible approach. Basically, I'm afraid that unilaterally renaming cube is going to break enough applications that there will be more people who flat out don't want this patch than there will be who get benefit from it, and we end up voting to revert the feature altogether. If you'd like to take that risk then feel free to charge full steam ahead, but don't say you were not warned. And don't bother arguing that CUBE is reserved according to the standard, because that will not make one damn bit of difference to the people who will be unhappy. 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] Hardening pg_upgrade
Bruce Momjian wrote I had to make an exception for temporary tables because pg_upgrade uses temporary tables to collect schema information. I tried writing the query to use CTEs (second patch), but I would then have to have one query for 8.3, which doesn't support CTEs, and another for 8.4+, plus the CTE query was more complex than I liked. Another idea would be to drop 8.3 support (and remove lots of code to support that), but the recent large increase in the number of people upgrading from 8.4 makes that unattractive. (8.3 did use a different timestamp storage format though.) Why not tell people on 8.3- that a direct upgrade is not supported but that an indirect upgrade to 9.4 or earlier has to be performed first and then that can be upgraded to 9.5+ ? I'm not clear on how the 8.4 upgrades volume impacts a decision to support 8.3- upgrades? David J. -- View this message in context: http://postgresql.1045698.n5.nabble.com/Hardening-pg-upgrade-tp5815735p5815748.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] WIP Patch for GROUPING SETS phase 1
On Thu, Aug 21, 2014 at 1:13 PM, Tom Lane t...@sss.pgh.pa.us wrote: Andrew Gierth and...@tao11.riddles.org.uk writes: Tom == Tom Lane t...@sss.pgh.pa.us writes: Tom I wonder if you've tried hard enough to avoid reserving the keyword. GROUP BY cube(a,b) is currently legal syntax and means something completely incompatible to what the spec requires. Well, if there are any extant applications that use that exact phrasing, they're going to be broken in any case. That does not mean that we have to break every other appearance of cube. I think that special-casing appearances of cube(...) in GROUP BY lists might be a feasible approach. Basically, I'm afraid that unilaterally renaming cube is going to break enough applications that there will be more people who flat out don't want this patch than there will be who get benefit from it, and we end up voting to revert the feature altogether. If you'd like to take that risk then feel free to charge full steam ahead, but don't say you were not warned. And don't bother arguing that CUBE is reserved according to the standard, because that will not make one damn bit of difference to the people who will be unhappy. I have to respectfully disagree. Certainly, if there is some reasonable way to not have to change 'cube' then great. But the tonnage rule applies here: even considering compatibility issues, when considering the importance of standard SQL (and, I might add, exceptionally useful) syntax and a niche extension, 'cube' is going to have to get out of the way. There are view valid reasons to break compatibility but blocking standard syntax is definitely one of them. merlin -- 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] [GSoC2014] Patch ALTER TABLE ... SET LOGGED
On 08/21/2014 05:04 PM, Thom Brown wrote: On 21 August 2014 14:45, Fabrízio de Royes Mello fabriziome...@gmail.com wrote: On Thu, Aug 21, 2014 at 5:23 AM, Christoph Berg c...@df7cb.de wrote: Re: Thom Brown 2014-08-20 CAA-aLv7TeF8iM= 7u7tsgl4s5jh1a+shq_ny7gorzc_g_yj7...@mail.gmail.com ERROR: table test is not permanent Perhaps this would be better as table test is unlogged as permanent doesn't match the term used in the DDL syntax. I was also wondering that, but then figured that when ALTER TABLE SET UNLOGGED is invoked on temp tables, the error message is not permanent was correct while the apparent opposite is unlogged is wrong. Thom, Christoph is right... make no sense the message... see the example: fabrizio=# create temp table foo(); CREATE TABLE fabrizio=# alter table foo set unlogged; ERROR: table foo is unlogged The previous message is better: fabrizio=# create temp table foo(); CREATE TABLE fabrizio=# alter table foo set unlogged; ERROR: table foo is not permanent fabrizio=# fabrizio=# create unlogged table foo2(); CREATE TABLE fabrizio=# alter table foo2 set unlogged; ERROR: table foo2 is not permanent To me, that's even more confusing: CREATE TEMP TABLE test(); CREATE UNLOGGED TABLE test2(); # ALTER TABLE test SET LOGGED; ERROR: table test is not unlogged # ALTER TABLE test SET UNLOGGED; ERROR: table test is not permanent # ALTER TABLE test2 SET UNLOGGED; ERROR: table test2 is not permanent They're being rejected for different reasons but the error message is identical. Permanent suggests the opposite of temporary, and unlogged tables aren't temporary. In Postgres internals slang, non-permanent means temporary or unlogged. But I agree we shouldn't expose users to that term; we use it in the docs, and it's not used in command names either. I wonder if throwing an error is correct behavior anyway. Other ALTER TABLE commands just silently do nothing in similar situations, e.g: lowerdb=# CREATE TABLE foo () WITH OIDS; CREATE TABLE lowerdb=# ALTER TABLE foo SET WITH OIDS; ALTER TABLE But if we want to throw an error anyway, I'd suggest phrasing it table foo is already unlogged - 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] Hardening pg_upgrade
On Thu, Aug 21, 2014 at 11:43:42AM -0700, David G Johnston wrote: Bruce Momjian wrote I had to make an exception for temporary tables because pg_upgrade uses temporary tables to collect schema information. I tried writing the query to use CTEs (second patch), but I would then have to have one query for 8.3, which doesn't support CTEs, and another for 8.4+, plus the CTE query was more complex than I liked. Another idea would be to drop 8.3 support (and remove lots of code to support that), but the recent large increase in the number of people upgrading from 8.4 makes that unattractive. (8.3 did use a different timestamp storage format though.) Why not tell people on 8.3- that a direct upgrade is not supported but that an indirect upgrade to 9.4 or earlier has to be performed first and then that can be upgraded to 9.5+ ? Yes, we could easily do that, and trim down pg_upgrade in the process. Are people OK with that? I'm not clear on how the 8.4 upgrades volume impacts a decision to support 8.3- upgrades? My point is that people aren't doing upgrades just from 9.1 and 9.2, but often from very old releases. and the end-of-lifed of 8.4 prompted a lot of people to upgrade. Now, since 8.3 has been end-of-lifed since February, 2013, we might be able to argue that 8.3 already had a year to upgrade, so if they now want to upgrade, they have to do it in two steps. Anyway, I think we need more opinions on this. -- 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] run xmllint during build (was Re: need xmllint on borka)
On 8/21/14 5:12 AM, Fabien COELHO wrote: However, it seems that it is not run for target html, nor for pdf/ps targets. I'm wondering whether it is an oversight or if there is a reason for that. Maybe the intention is that an explicit htmlhelp is expected to do the checking, but then why force it for man and epub? It seems to me that it is not consistent. It is only run when the build is via XML/XSLT, not via SGML/DSSSL (because the SGML/DSSSL build already checks the validity anyway). -- 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] run xmllint during build (was Re: need xmllint on borka)
On 8/21/14 5:12 AM, Fabien COELHO wrote: Also, a general comment, which is independent of this patch: I found the documentation build especially not resilient, with a lack of clear error messages when something is broken. Basically, if configure does not found something for the doc (openjade, osx, xmllint, ...) it does not complain. That is fine with me, people would not always want to build the doc anyway as it is available online. However, the Makefile in doc/src/sgml overrides the not found commands (ifndef JADE JADE=..., etc), and proceed to unhelpful and unclear errors later on. ISTM that it may be more helful to do: ifndef JADE #error no jade found on your system, cannot generate the documention endif We could use $(missing) for that, which is already used for bison, flex, and perl. -- 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] Enable WAL archiving even in standby
On Tue, Aug 19, 2014 at 7:33 AM, Fujii Masao masao.fu...@gmail.com wrote: On Fri, Aug 15, 2014 at 4:30 AM, Robert Haas robertmh...@gmail.com wrote: On Wed, Aug 13, 2014 at 6:42 AM, Fujii Masao masao.fu...@gmail.com wrote: I'd propose the attached WIP patch which allows us to enable WAL archiving even in standby. The patch adds always as the valid value of archive_mode. If it's set to always, the archiver is started when the server is in standby mode and all the WAL files that walreceiver wrote to the disk are archived by using archive_command. Then, even after the server is promoted to master, the archiver keeps archiving WAL files. The patch doesn't change the meanings of the setting values on and off of archive_mode. I like the feature, but I don't much like this as a control mechanism. Having archive_command and standby_archive_command, as you propose further down, seems saner. Okay, that's fine. One question is; Which WAL files should be archived by standby_archive_command? There are following kinds of WAL files. (1) WAL files which were fully written and closed by walreceiver Curently they are not archived at all. (2) WAL file which is being written by walreceiver This file will be closed before it's fully written because of, for example, standby promotion. Currently this is archived by archive_command. (3) WAL file with new timeline, which is copied from (2) At the end of recovery, after new timeline is assigned, this latest WAL file with new timeline is created by being copied from (2) (i.e., latest WAL file with old timeline). WAL data of end-of-recovery checkpoint is written to this latest WAL file. Currently this is archived by archive_command. (4) Timeline history files When standby is promoted to the master, the imeline is incremented and the timeline history file is created. Currently the timeline history files are archived by archive_command. (5) WAL files generated in normal processing mode Currently they are archived by archive_command. I'm thinking to use standby_archive_command only for (1) because the others are currently archived by archive_command. That means that even if there are type (1) WAL files which have not been archived yet after the standby promotion (i.e., the situation where WAL archiving was delayed for some reasons in the standby), they are archived by standby_archive_command. IOW, the archiver uses both archive commands as the situation demands. OTOH, maybe there are people who want to use standby_archive_command for all the WAL files with old timeline, i.e., (1) and (2). Thought? Boy, that's quite confusing. I didn't think we ever ran archive_command on the standby right now, so then it would make sense to have a way to do that. And it makes sense for it to be separate from the mode used on the master to avoid breaking existing configurations, so that a user assuming that a certain setting will only take effect after promotion will not be disappointed. However, if what you're saying is that we do archiving on the standby sometimes but not others, I'm not quite sure what the best way forward is. It seems rather inconsistent to do it for some types of WAL files but not others. -- 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] run xmllint during build (was Re: need xmllint on borka)
Peter Eisentraut pete...@gmx.net writes: On 8/21/14 5:12 AM, Fabien COELHO wrote: However, it seems that it is not run for target html, nor for pdf/ps targets. I'm wondering whether it is an oversight or if there is a reason for that. Maybe the intention is that an explicit htmlhelp is expected to do the checking, but then why force it for man and epub? It seems to me that it is not consistent. It is only run when the build is via XML/XSLT, not via SGML/DSSSL (because the SGML/DSSSL build already checks the validity anyway). I'm confused. I thought the point of this change was mostly that the SGML toolchain is less strict than the XML toolchain, and you wanted to have the more-strict checks applied whenever possible. 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] proposal: rounding up time value less than its unit.
On 8/21/14 11:16 AM, Tom Lane wrote: Heikki Linnakangas hlinnakan...@vmware.com writes: The patch also rounds a zero up to one. A naked zero with no unit is not affected, but e.g if you have log_rotation_age=0s, it will not disable the feature as you might expect, but set it to 1 minute. Should we do something about that? That sounds like a dealbreaker to me. There are enough places where zero has special meaning that we should not *ever* change zero to non-zero silently. I don't think I like this idea anyway. If something has units of an hour and the user (perhaps misunderstanding the setting) sets it to one second, then we shouldn't silently change that to one hour. If there is a problem with rounding it to zero, then we should perhaps raise an error. (And stop treating zero specially. It's a terrible idea.) -- 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] run xmllint during build (was Re: need xmllint on borka)
Peter Eisentraut pete...@gmx.net writes: We could use $(missing) for that, which is already used for bison, flex, and perl. +1 ... I'm surprised it's not like that already. Fabien's quite right to complain that the Makefile has no business inserting defaults for things configure couldn't find. 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] Hardening pg_upgrade
Bruce Momjian br...@momjian.us writes: Specifically, the first attached patch causes pg_upgrade_support functions to throw errors when called by a backend that is not in binary upgrade mode. (This seems like a good safety measure.) Agreed about that part. Second, and more importantly, the patch prevents automatic oid assignment when in binary upgrade mode, except for temporary objects. This is to help guarantee that system-assigned oids do not conflict with preserved oids. I had to make an exception for temporary tables because pg_upgrade uses temporary tables to collect schema information. This seems like a bad idea. If you are going to have such an off-switch at all (which I'm not sure I buy the need for), it should not have holes in it. I tried writing the query to use CTEs (second patch), but I would then have to have one query for 8.3, which doesn't support CTEs, and another for 8.4+, plus the CTE query was more complex than I liked. Another idea would be to drop 8.3 support (and remove lots of code to support that), but the recent large increase in the number of people upgrading from 8.4 makes that unattractive. (8.3 did use a different timestamp storage format though.) I vote for discarding 8.3 support in pg_upgrade. There are already enough limitations on pg_upgrade from pre-8.4 to make it of questionable value; if it's going to create problems like this, it's time to cut the rope. 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] Hardening pg_upgrade
On Thu, Aug 21, 2014 at 10:07 PM, Tom Lane t...@sss.pgh.pa.us wrote: Bruce Momjian br...@momjian.us writes: I tried writing the query to use CTEs (second patch), but I would then have to have one query for 8.3, which doesn't support CTEs, and another for 8.4+, plus the CTE query was more complex than I liked. Another idea would be to drop 8.3 support (and remove lots of code to support that), but the recent large increase in the number of people upgrading from 8.4 makes that unattractive. (8.3 did use a different timestamp storage format though.) I vote for discarding 8.3 support in pg_upgrade. There are already enough limitations on pg_upgrade from pre-8.4 to make it of questionable value; if it's going to create problems like this, it's time to cut the rope. +1. 8.3 has been unsupported for a fairly long time now, and you can still do a two-step upgrade if you're on that old a version. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.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] run xmllint during build (was Re: need xmllint on borka)
On 8/21/14 4:00 PM, Tom Lane wrote: It is only run when the build is via XML/XSLT, not via SGML/DSSSL (because the SGML/DSSSL build already checks the validity anyway). I'm confused. I thought the point of this change was mostly that the SGML toolchain is less strict than the XML toolchain, and you wanted to have the more-strict checks applied whenever possible. The SGML tool chain is less strict about what it considers valid, but the XML toolchain doesn't check at all unless we run xmllint, it just produces garbage when the input is invalid. -- 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] run xmllint during build (was Re: need xmllint on borka)
It is only run when the build is via XML/XSLT, not via SGML/DSSSL (because the SGML/DSSSL build already checks the validity anyway). I'm confused. I thought the point of this change was mostly that the SGML toolchain is less strict than the XML toolchain, and you wanted to have the more-strict checks applied whenever possible. The SGML tool chain is less strict about what it considers valid, but the XML toolchain doesn't check at all unless we run xmllint, it just produces garbage when the input is invalid. This suggests that xmllint should always be run, because it is more strict than the other, so it is safer if the source has passed it and is validated, whatever the tool chain used afterwards? -- Fabien. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] run xmllint during build (was Re: need xmllint on borka)
ISTM that it may be more helful to do: ifndef JADE #error no jade found on your system, cannot generate the documention endif We could use $(missing) for that, which is already used for bison, flex, and perl. I'm fine with $(missing) or whatever else that provides a clear message. Oops, not #error, but $(error ...), I was mixing cpp make above... However the example in the doc Makefile for collageindex.pl is on the heavy side, as it suggests that every use of such commands should be put in ifdef/else/endif. ISTM that a dependence-based solution would be simpler. -- Fabien. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP Patch for GROUPING SETS phase 1
On 08/21/2014 02:48 PM, Merlin Moncure wrote: Basically, I'm afraid that unilaterally renaming cube is going to break enough applications that there will be more people who flat out don't want this patch than there will be who get benefit from it, and we end up voting to revert the feature altogether. If you'd like to take that risk then feel free to charge full steam ahead, but don't say you were not warned. And don't bother arguing that CUBE is reserved according to the standard, because that will not make one damn bit of difference to the people who will be unhappy. I have to respectfully disagree. Certainly, if there is some reasonable way to not have to change 'cube' then great. But the tonnage rule applies here: even considering compatibility issues, when considering the importance of standard SQL (and, I might add, exceptionally useful) syntax and a niche extension, 'cube' is going to have to get out of the way. There are view valid reasons to break compatibility but blocking standard syntax is definitely one of them. I'm inclined to think that the audience for this is far larger than the audience for the cube extension, which I have not once encountered in the field. But I guess we all have different experiences. 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] proposal: rounding up time value less than its unit.
Peter Eisentraut-2 wrote On 8/21/14 11:16 AM, Tom Lane wrote: Heikki Linnakangas lt; hlinnakangas@ gt; writes: The patch also rounds a zero up to one. A naked zero with no unit is not affected, but e.g if you have log_rotation_age=0s, it will not disable the feature as you might expect, but set it to 1 minute. Should we do something about that? That sounds like a dealbreaker to me. There are enough places where zero has special meaning that we should not *ever* change zero to non-zero silently. I don't think I like this idea anyway. If something has units of an hour and the user (perhaps misunderstanding the setting) sets it to one second, then we shouldn't silently change that to one hour. If there is a problem with rounding it to zero, then we should perhaps raise an error. (And stop treating zero specially. It's a terrible idea.) I'm on board, from the original thread, for errors if the input cannot be converted to the parameter measurement unit cleanly. By which I mean the specified value should result in an integer being recorded without rounding. Specifying a precision less than the default unit thus becomes impossible. I don't have a problem with zero meaning disabled when appropriate since it avoids having a separate on/off GUC. That said the complaint here just seems like a bug in the supplied patch - zero is zero regardless of whether a unit is specified. The only obvious exception would be temperature but that isn't relevant here. David J. -- View this message in context: http://postgresql.1045698.n5.nabble.com/proposal-rounding-up-time-value-less-than-its-unit-tp5811102p5815770.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] [GSoC2014] Patch ALTER TABLE ... SET LOGGED
Heikki Linnakangas wrote:
In Postgres internals slang, non-permanent means temporary or
unlogged. But I agree we shouldn't expose users to that term; we use
it in the docs, and it's not used in command names either.
Agreed. I am going over this patch, and the last bit I need to sort out
is the wording of these messages. I have temporarily settled on this:
ereport(ERROR,
(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
errmsg(cannot change logged status of table %s to
logged,
RelationGetRelationName(rel)),
errdetail(Table %s references unlogged table %s.,
RelationGetRelationName(rel),
RelationGetRelationName(relfk;
Note the term logged status to talk about whether a table is logged or
not. I thought about loggedness but I'm not sure english speakers are
going to love me for that. Any other ideas there?
I wonder if throwing an error is correct behavior anyway. Other
ALTER TABLE commands just silently do nothing in similar situations,
e.g:
lowerdb=# CREATE TABLE foo () WITH OIDS;
CREATE TABLE
lowerdb=# ALTER TABLE foo SET WITH OIDS;
ALTER TABLE
But if we want to throw an error anyway, I'd suggest phrasing it
table foo is already unlogged
Yeah, there is precedent for silently doing nothing. We previously
threw warnings or notices, but nowadays even that is gone. Throwing an
error definitely seems the wrong thing. In the patch I have it's like
this:
ereport(ERROR,
(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
errmsg(cannot change logged status of table %s to
unlogged,
RelationGetRelationName(rel)),
errdetail(Table %s is already unlogged.,
RelationGetRelationName(rel;
but I think I will just take that out as a whole and set a flag to
indicate nothing is to be done.
(This also means that setting tab-rewrite while analyzing the command
is the wrong thing to do. Instead, the test for tab-rewrite should
have an || tab-chgLoggedness test, and we set chgLoggedness off if we
see that it's a no-op. That way we avoid a pointless table rewrite and
a pointless error in a multi-command ALTER TABLE that has a no-op SET
LOGGED subcommand among other things.)
I changed the doc item in ALTER TABLE,
varlistentry
termliteralSET {LOGGED | UNLOGGED}/literal/term
listitem
para
This form changes the table from unlogged to logged or vice-versa
(see xref linkend=SQL-CREATETABLE-UNLOGGED). It cannot be applied
to a temporary table.
/para
/listitem
/varlistentry
I removed the fact that it needs ACCESS EXCLUSIVE because that's already
mentioned in the introductory paragraph. I also removed the phrase that
it requires a table rewrite, because on reading existing text I noticed
that we don't document which forms cause rewrites. Perhaps we should
document that somehow, but adding it to only one item seems wrong.
I will post an updated patch as soon as I fix a bug I introduced in the
check for FKs.
--
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP Patch for GROUPING SETS phase 1
Andrew Dunstan and...@dunslane.net writes: I'm inclined to think that the audience for this is far larger than the audience for the cube extension, which I have not once encountered in the field. Perhaps so. I would really prefer not to have to get into estimating how many people will be inconvenienced how badly. It's clear to me that not a lot of sweat has been put into seeing if we can avoid reserving the keyword, and I think we need to put in that effort. We've jumped through some pretty high hoops to avoid reserving keywords in the past, so I don't think this patch should get a free pass on the issue. Especially considering that renaming the cube extension isn't exactly going to be zero work: there is no infrastructure for such a thing. A patch consisting merely of s/cube/foobar/g isn't going to cut it. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [GSoC2014] Patch ALTER TABLE ... SET LOGGED
Alvaro Herrera alvhe...@2ndquadrant.com writes: Agreed. I am going over this patch, and the last bit I need to sort out is the wording of these messages. I have temporarily settled on this: ereport(ERROR, (errcode(ERRCODE_INVALID_TABLE_DEFINITION), errmsg(cannot change logged status of table %s to logged, RelationGetRelationName(rel)), errdetail(Table %s references unlogged table %s., RelationGetRelationName(rel), RelationGetRelationName(relfk; Note the term logged status to talk about whether a table is logged or not. I thought about loggedness but I'm not sure english speakers are going to love me for that. Any other ideas there? Just say cannot change status of table %s to logged. Yeah, there is precedent for silently doing nothing. We previously threw warnings or notices, but nowadays even that is gone. Throwing an error definitely seems the wrong thing. Agreed, just do nothing if it's already the right setting. 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] [GSoC2014] Patch ALTER TABLE ... SET LOGGED
On 2014-08-21 16:59:17 -0400, Alvaro Herrera wrote: Heikki Linnakangas wrote: In Postgres internals slang, non-permanent means temporary or unlogged. But I agree we shouldn't expose users to that term; we use it in the docs, and it's not used in command names either. Agreed. I am going over this patch, and the last bit I need to sort out is the wording of these messages. I have temporarily settled on this: ereport(ERROR, (errcode(ERRCODE_INVALID_TABLE_DEFINITION), errmsg(cannot change logged status of table %s to logged, RelationGetRelationName(rel)), errdetail(Table %s references unlogged table %s., RelationGetRelationName(rel), RelationGetRelationName(relfk; Maybe 'cannot change persistency of table .. from unlogged to logged'; possibly with s/persistency/durability/? Have you looked at the correctness of the patch itself? Last time I'd looked it has fundamental correctness issues. I'd outlined a possible solution, but I haven't looked since. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] jsonb format is pessimal for toast compression
On 08/20/2014 03:42 PM, Arthur Silva wrote: What data are you using right now Josh? The same data as upthread. Can you test the three patches (9.4 head, 9.4 with Tom's cleanup of Heikki's patch, and 9.4 with Tom's latest lengths-only) on your workload? I'm concerned that my workload is unusual and don't want us to make this decision based entirely on it. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add the number of pinning backends to pg_buffercache's output
On 2014-07-02 08:55:05 +, Rajeev rastogi wrote: Also can we change the description of function pg_buffercache_pages to include pinning_backend also in the description. I'm not sure what you mean here - I've adjusted the docs to include the column? Or do you mean that it errorneously was named pincount as Fujii noticed? Thanks for the review, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP Patch for GROUPING SETS phase 1
* Tom Lane (t...@sss.pgh.pa.us) wrote: Andrew Dunstan and...@dunslane.net writes: I'm inclined to think that the audience for this is far larger than the audience for the cube extension, which I have not once encountered in the field. +1 Perhaps so. I would really prefer not to have to get into estimating how many people will be inconvenienced how badly. It's clear to me that not a lot of sweat has been put into seeing if we can avoid reserving the keyword, and I think we need to put in that effort. I'm with Merlin on this one, it's going to end up happening and I don't know that 9.5 is any worse than post-9.5 to make this change. We've jumped through some pretty high hoops to avoid reserving keywords in the past, so I don't think this patch should get a free pass on the issue. This doesn't feel like an attempt to get a free pass on anything- it's not being proposed as fully reserved and there is spec-defined syntax which needs to be supported. If we can get away with keeping it unreserved while not making it utterly confusing for users and convoluting the code, great, but that doesn't seem likely to pan out. Especially considering that renaming the cube extension isn't exactly going to be zero work: there is no infrastructure for such a thing. A patch consisting merely of s/cube/foobar/g isn't going to cut it. This is a much more interesting challenge to deal with, but perhaps we could include a perl script or pg_upgrade snippet for users to run to see if they have the extension installed and to do some magic before the actual upgrade to handle the rename..? Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] pg_upgrade: allow multiple -o/-O options
On Tue, Mar 4, 2014 at 04:52:56PM +0100, Pavel Raiskup wrote:
Hello,
RFE: Consider that you want to run pg_upgrade via some script with some
default '-o' option. But then you also want to give the script's user a
chance to specify the old-server's options according user's needs.
Then something like the following is not possible:
$ cat script
...
pg_upgrade ... -o 'sth' $PG_UPGRADE_OPT ...
...
I know that this problem is still script-able, but the fix should be
innocent and it would simplify things. Thanks for considering,
Attached is a patch that makes multiple -o options append their
arguments for pg_upgrade and pg_ctl, and documents this and the append
behavior of postmaster/postgres. This covers all the -o behaviors.
--
Bruce Momjian br...@momjian.ushttp://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
diff --git a/contrib/pg_upgrade/option.c b/contrib/pg_upgrade/option.c
new file mode 100644
index bb594dd..cfc88ec
*** a/contrib/pg_upgrade/option.c
--- b/contrib/pg_upgrade/option.c
*** parseCommandLine(int argc, char *argv[])
*** 137,153
break;
case 'o':
! old_cluster.pgopts = pg_strdup(optarg);
break;
case 'O':
! new_cluster.pgopts = pg_strdup(optarg);
break;
/*
* Someday, the port number option could be removed and passed
* using -o/-O, but that requires postmaster -C to be
! * supported on all old/new versions.
*/
case 'p':
if ((old_cluster.port = atoi(optarg)) = 0)
--- 137,171
break;
case 'o':
! /* append option? */
! if (!old_cluster.pgopts)
! old_cluster.pgopts = pg_strdup(optarg);
! else
! {
! char *old_pgopts = old_cluster.pgopts;
!
! old_cluster.pgopts = psprintf(%s %s, old_pgopts, optarg);
! free(old_pgopts);
! }
break;
case 'O':
! /* append option? */
! if (!new_cluster.pgopts)
! new_cluster.pgopts = pg_strdup(optarg);
! else
! {
! char *new_pgopts = new_cluster.pgopts;
!
! new_cluster.pgopts = psprintf(%s %s, new_pgopts, optarg);
! free(new_pgopts);
! }
break;
/*
* Someday, the port number option could be removed and passed
* using -o/-O, but that requires postmaster -C to be
! * supported on all old/new versions (added in PG 9.2).
*/
case 'p':
if ((old_cluster.port = atoi(optarg)) = 0)
diff --git a/doc/src/sgml/pgupgrade.sgml b/doc/src/sgml/pgupgrade.sgml
new file mode 100644
index b79f0db..dd57c5c
*** a/doc/src/sgml/pgupgrade.sgml
--- b/doc/src/sgml/pgupgrade.sgml
***
*** 130,143
termoption-o/option replaceable class=parameteroptions/replaceable/term
termoption--old-options/option replaceable class=parameteroptions/replaceable/term
listitemparaoptions to be passed directly to the
! old commandpostgres/command command/para/listitem
/varlistentry
varlistentry
termoption-O/option replaceable class=parameteroptions/replaceable/term
termoption--new-options/option replaceable class=parameteroptions/replaceable/term
listitemparaoptions to be passed directly to the
! new commandpostgres/command command/para/listitem
/varlistentry
varlistentry
--- 130,145
termoption-o/option replaceable class=parameteroptions/replaceable/term
termoption--old-options/option replaceable class=parameteroptions/replaceable/term
listitemparaoptions to be passed directly to the
! old commandpostgres/command command; multiple
! option invocations are appended/para/listitem
/varlistentry
varlistentry
termoption-O/option replaceable class=parameteroptions/replaceable/term
termoption--new-options/option replaceable class=parameteroptions/replaceable/term
listitemparaoptions to be passed directly to the
! new commandpostgres/command command; multiple
! option invocations are appended/para/listitem
/varlistentry
varlistentry
diff --git a/doc/src/sgml/ref/pg_ctl-ref.sgml b/doc/src/sgml/ref/pg_ctl-ref.sgml
new file mode 100644
index 2368129..29f882b
*** a/doc/src/sgml/ref/pg_ctl-ref.sgml
--- b/doc/src/sgml/ref/pg_ctl-ref.sgml
*** PostgreSQL documentation
*** 302,308
listitem
para
Specifies options to be passed directly to the
! commandpostgres/command command.
/para
para
The options should usually be surrounded by single or double
--- 302,309
listitem
para
Specifies options to be passed directly to the
! commandpostgres/command command; multiple
! option invocations are appended.
/para
para
The options should usually be surrounded by single or double
diff --git
Re: [HACKERS] [GSoC2014] Patch ALTER TABLE ... SET LOGGED
Tom Lane wrote:
Alvaro Herrera alvhe...@2ndquadrant.com writes:
Agreed. I am going over this patch, and the last bit I need to sort out
is the wording of these messages. I have temporarily settled on this:
ereport(ERROR,
(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
errmsg(cannot change logged status of table %s to
logged,
RelationGetRelationName(rel)),
errdetail(Table %s references unlogged table %s.,
RelationGetRelationName(rel),
RelationGetRelationName(relfk;
Note the term logged status to talk about whether a table is logged or
not. I thought about loggedness but I'm not sure english speakers are
going to love me for that. Any other ideas there?
Just say cannot change status of table %s to logged.
I like this one, thanks.
Yeah, there is precedent for silently doing nothing. We previously
threw warnings or notices, but nowadays even that is gone. Throwing an
error definitely seems the wrong thing.
Agreed, just do nothing if it's already the right setting.
Done that way.
Andres Freund wrote:
Have you looked at the correctness of the patch itself? Last time I'd
looked it has fundamental correctness issues. I'd outlined a possible
solution, but I haven't looked since.
Yeah, Fabrizio had it passing the relpersistence down to make_new_heap,
so the transient table is created with the right setting. AFAICS it's
good now. I'm a bit uneasy about the way it changes indexes: it just
updates pg_class for them just before invoking the reindex in
finish_heap_swap. I think it's correct as it stands though; at least,
the rewrite phase happens with the right setting, so that if there are
constraints being checked and these invoke index scans, such accesses
would not leave buffers with the wrong setting in shared_buffers.
Another option would be to pass the new relpersistence down to
finish_heap_swap, but that would be hugely complicated AFAICS.
Here's the updated patch.
--
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
*** a/doc/src/sgml/ref/alter_table.sgml
--- b/doc/src/sgml/ref/alter_table.sgml
***
*** 61,66 ALTER TABLE [ IF EXISTS ] replaceable class=PARAMETERname/replaceable
--- 61,68
SET WITHOUT CLUSTER
SET WITH OIDS
SET WITHOUT OIDS
+ SET TABLESPACE replaceable class=PARAMETERnew_tablespace/replaceable
+ SET {LOGGED | UNLOGGED}
SET ( replaceable class=PARAMETERstorage_parameter/replaceable = replaceable class=PARAMETERvalue/replaceable [, ... ] )
RESET ( replaceable class=PARAMETERstorage_parameter/replaceable [, ... ] )
INHERIT replaceable class=PARAMETERparent_table/replaceable
***
*** 68,74 ALTER TABLE [ IF EXISTS ] replaceable class=PARAMETERname/replaceable
OF replaceable class=PARAMETERtype_name/replaceable
NOT OF
OWNER TO replaceable class=PARAMETERnew_owner/replaceable
- SET TABLESPACE replaceable class=PARAMETERnew_tablespace/replaceable
REPLICA IDENTITY {DEFAULT | USING INDEX replaceable class=PARAMETERindex_name/replaceable | FULL | NOTHING}
phraseand replaceable class=PARAMETERtable_constraint_using_index/replaceable is:/phrase
--- 70,75
***
*** 477,482 ALTER TABLE [ IF EXISTS ] replaceable class=PARAMETERname/replaceable
--- 478,508
/varlistentry
varlistentry
+ termliteralSET TABLESPACE/literal/term
+ listitem
+ para
+ This form changes the table's tablespace to the specified tablespace and
+ moves the data file(s) associated with the table to the new tablespace.
+ Indexes on the table, if any, are not moved; but they can be moved
+ separately with additional literalSET TABLESPACE/literal commands.
+ See also
+ xref linkend=SQL-CREATETABLESPACE.
+ /para
+ /listitem
+/varlistentry
+
+varlistentry
+ termliteralSET {LOGGED | UNLOGGED}/literal/term
+ listitem
+ para
+ This form changes the table from unlogged to logged or vice-versa
+ (see xref linkend=SQL-CREATETABLE-UNLOGGED). It cannot be applied
+ to a temporary table.
+ /para
+ /listitem
+/varlistentry
+
+varlistentry
termliteralSET ( replaceable class=PARAMETERstorage_parameter/replaceable = replaceable class=PARAMETERvalue/replaceable [, ... ] )/literal/term
listitem
para
***
*** 589,608 ALTER TABLE [ IF EXISTS ] replaceable class=PARAMETERname/replaceable
/listitem
/varlistentry
-varlistentry
- termliteralSET TABLESPACE/literal/term
- listitem
- para
- This form changes the table's tablespace to the specified tablespace and
- moves the data file(s) associated with the table to the new
Re: [HACKERS] WIP Patch for GROUPING SETS phase 1
On Thu, Aug 21, 2014 at 06:15:33PM -0400, Stephen Frost wrote: * Tom Lane (t...@sss.pgh.pa.us) wrote: Andrew Dunstan and...@dunslane.net writes: I'm inclined to think that the audience for this is far larger than the audience for the cube extension, which I have not once encountered in the field. +1 I haven't seen it in the field either. I'd also like to mention that the mere presence of a module in our contrib/ directory can reflect bad decisions that need reversing just as easily as it can the presence of vitally important utilities that need to be preserved. I'm pretty sure the cube extension arrived after the CUBE keyword in SQL, which makes that an error on our part if true. Especially considering that renaming the cube extension isn't exactly going to be zero work: there is no infrastructure for such a thing. A patch consisting merely of s/cube/foobar/g isn't going to cut it. This is a much more interesting challenge to deal with, but perhaps we could include a perl script or pg_upgrade snippet for users to run to see if they have the extension installed and to do some magic before the actual upgrade to handle the rename..? +1 for doing this. Do we want to make some kind of generator for such things? It doesn't seem hard in principle, but I haven't tried coding it up yet. Cheers, David. -- David Fetter da...@fetter.org http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- 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] Should extension--unpackaged-$ver.sql's \echo use FROM unpackaged;?
On 2014-07-06 16:58:33 -0400, Robert Haas wrote: On Thu, Jul 3, 2014 at 9:03 AM, Fujii Masao masao.fu...@gmail.com wrote: On Thu, Jul 3, 2014 at 4:26 AM, Andres Freund and...@2ndquadrant.com wrote: Hi, Fujii noticed that I'd used \echo Use CREATE EXTENSION pg_buffercache to load this file. \quit in an extension upgrade script. That's obviously wrong, because it should say ALTER EXTENSION. The reason it's that way is that I copied the line from the unpackaged--1.0.sql file. I noticed all unpackaged--$version transition files miss the FROM unpackaged in that echo. I guess that's just a oversight which we should correct? Maybe for user-friendness. But I think that's not so important fix enough to backpatch. Seems like a clear mistake to me. +1 for fixing it and back-patching. I think so as well - and I now have a patch for it. Fujii, do you mind if I backpatch 6f9e39bc9993c1 to 9.1, 9.2 and 9.3 for you? Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER TABLESPACE MOVE command tag tweak
Alvaro, * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: ALTER TABLE ALL IN TABLESPACE xyz which AFAICS should work since ALL is already a reserved keyword. Pushed to master and REL9_4_STABLE. Apologies on it taking so long- things have a bit interesting for me over the past month or two. :) Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] Is this a bug?
On Tue, Mar 18, 2014 at 09:11:46AM -0400, Robert Haas wrote: On Mon, Mar 17, 2014 at 10:27 PM, Michael Paquier michael.paqu...@gmail.com wrote: On Tue, Mar 18, 2014 at 10:24 AM, Fabrízio de Royes Mello fabriziome...@gmail.com wrote: On Thu, Mar 13, 2014 at 10:22 AM, Robert Haas robertmh...@gmail.com wrote: Well, it's fairly harmless, but it might not be a bad idea to tighten that up. The attached patch tighten that up. Hm... It might be interesting to include it in 9.4 IMO, somewhat grouping with what has been done in a6542a4 for SET and ABORT. Meh. There will always be another thing we could squeeze in; I don't think this is particularly urgent, and it's late to the party. Do we want this patch for 9.5? It throws an error for invalid reloption specifications. -- 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] pgcrypto: PGP signatures
Hi Marko,
I took a quick look at your patch at
http://www.postgresql.org/message-id/53edbcf0.9070...@joh.to (sorry I
didn't reply directly as I didn't have the message). It applies
cleanly, builds, and the tests pass. I will hopefully have more to
say after I've poked at it and understood it better, but in the
meantime a couple of trivial things I spotted:
In pgp-pgsql.c, function pgp_sym_decrypt_verify_bytea:
+ if (PG_NARGS() 3)
+ PG_FREE_IF_COPY(arg, 3);
+ if (PG_NARGS() 4)
+ PG_FREE_IF_COPY(arg, 4);
I think the first 'arg' should be 'psw'.
I think the same mistake appears in pgp_sym_decrypt_verify_text.
+ if (!sig-onepass)
+ {
+ time_t t;
+
+ isnull[3] = false;
+ /* unsigned big endian */
+ t = sig-creation_time[0] 24;
+ t += sig-creation_time[1] 16;
+ t += sig-creation_time[2] 8;
+ t += sig-creation_time[3];
+ values[3] = time_t_to_timestamptz(t);
+ }
Should t be of type pg_time_t rather than time_t? Would it be better
if PGP_Signature's member creation_time were of type uint32_t and you
could use ntohl(sig-creation_time) instead of the bitshifting?
In pgp.h:
+#define PGP_MAX_KEY(256/8)
+#define PGP_MAX_BLOCK (256/8)
+#define PGP_MAX_DIGEST (512/8)
+#define PGP_MAX_DIGEST_ASN1_PREFIX 20
+#define PGP_S2K_SALT 8
The PGP_MAX_DIGEST_ASN1_PREFIX line is new but the others just have
whitespace changes, and I gather the done thing is not to reformat
existing lines like that to distract from the changes in a patch.
(Just curious, why do you use while (1) for an infinite loop in
extract_signatures, but for (;;) in pullf_discard? It looks like the
latter is much more common in the source tree.)
Best regards,
Thomas Munro
--
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] [GSoC2014] Patch ALTER TABLE ... SET LOGGED
On Thu, Aug 21, 2014 at 8:04 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Andres Freund wrote: Have you looked at the correctness of the patch itself? Last time I'd looked it has fundamental correctness issues. I'd outlined a possible solution, but I haven't looked since. Yeah, Fabrizio had it passing the relpersistence down to make_new_heap, so the transient table is created with the right setting. AFAICS it's good now. I'm a bit uneasy about the way it changes indexes: it just updates pg_class for them just before invoking the reindex in finish_heap_swap. I think it's correct as it stands though; at least, the rewrite phase happens with the right setting, so that if there are constraints being checked and these invoke index scans, such accesses would not leave buffers with the wrong setting in shared_buffers. Ok. Another option would be to pass the new relpersistence down to finish_heap_swap, but that would be hugely complicated AFAICS. I think isn't so complicated to do it, but will this improve something ? Maybe I didn't understand it very well. IMHO it just complicate a simple thing. Here's the updated patch. Thanks Alvaro! 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
Re: [HACKERS] WIP Patch for GROUPING SETS phase 1
Stephen == Stephen Frost sfr...@snowman.net writes: I'm inclined to think that the audience for this is far larger than the audience for the cube extension, which I have not once encountered in the field. Stephen +1 Most of my encounters with cube have been me suggesting it to people on IRC as a possible approach for solving certain kinds of performance problems by converting them to N-dimensional spatial containment queries. Sometimes this works well, but it doesn't seem to be an approach that many people discover on their own. We've jumped through some pretty high hoops to avoid reserving keywords in the past, so I don't think this patch should get a free pass on the issue. Stephen This doesn't feel like an attempt to get a free pass on Stephen anything- it's not being proposed as fully reserved and Stephen there is spec-defined syntax which needs to be supported. Stephen If we can get away with keeping it unreserved while not Stephen making it utterly confusing for users and convoluting the Stephen code, great, but that doesn't seem likely to pan out. Having now spent some more time looking, I believe there is a solution which makes it unreserved which does not require any significant pain in the code. I'm not entirely convinced that this is the right approach in the long term, but it might allow for a more planned transition. The absolute minimum seems to be: GROUPING as a col_name_keyword (since GROUPING(x,y,...) in the select list as a grouping operation looks like a function call for any argument types) CUBE, ROLLUP, SETS as unreserved_keyword -- Andrew (irc:RhodiumToad) -- 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] New Model For Role Attributes and Fine Grained Permssions
Heikki, * Heikki Linnakangas (hlinnakan...@vmware.com) wrote: On 08/19/2014 04:27 AM, Brightwell, Adam wrote: This is a proof-of-concept patch for a new model around role attributes and fine grained permissions meant to alleviate the current over dependence on superuser. Hmm. How does this get us any closer to fine-grained permissions? This patch, by itself, does not- but it adds the structure to allow us to add more permissions without having to add more fields to pg_authid, and we could build into pg_permission the WITH ADMIN OPTION and grantor bits that the normal GRANT syntax already supports- but without having to chew up additional bits for these granted permissions which are applied to roles instead of objects in the system. As for specific examples where this could be used beyond those presented, consider things like: CREATE EXTENSION CREATE TABLESPACE COPY (server-side) The question which I've been kicking around is the possible additional constraints which we may want/need for these- a list of extensions which the role is allowed to create, a set of directories which the role is allowed to create tablespaces within, similairly a set of directories the role is allowed to use server-side COPY with (and perhaps a distinction between COPY FROM and COPY TO). I guess we need this, so that you can grant/revoke the permissions, but I thought the hard part is defining what the fine-grained permissions are, in a way that you can't escalate yourself to full superuser through any of them. This is definitely a concern- which is why I mention the specific items above as needing to be constrained in some fashion. CREATE EXTENSION and CREATE TABLESPACE are, in a way, naturally constrained if you imagine an environment where the user with those permissions doesn't have direct access to modify the filesystem. COPY, on the other hand, would allow the user to gain access to pg_authid through indirect means and therefore gain access to an actual superuser role on the system, potentially. (Ok- it might be possible to do that with CREATE TABLESPACE too, but it'd be a bit more interesting to work through that than being able to just COPY to a bytea and download the result) The syntax for GRANT/REVOKE is quite complicated already. Do we want to overload it even more? Also keep in mind that the SQL standard specifies GRANT/REVOKE, so we have to be careful to not clash with the SQL standard syntax, or any conceivable future syntax the SQL committee might add to it (although we have plenty of PostgreSQL extensions in it already). For example, this is currently legal: I agree that there are concerns in this area and I've not got any great solutions. There are certainly pros and cons to using GRANT. It's familiar and natural to DBAs, but it runs the risk of conflicting with future SQL syntax, or even our own GRANT role. We can avoid the latter by keeping to reserved keywords only, but that may lead to some rather odd syntax (how would the GRANT COPY ON (dir1, dir2, dir3) work?). Is there a good alternative to consider though..? Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] inherit support for foreign tables
On Wed, Aug 20, 2014 at 08:11:01PM +0900, Etsuro Fujita wrote: (2014/07/02 11:23), Noah Misch wrote: Your chosen ANALYZE behavior is fair, but the messaging from a database-wide ANALYZE VERBOSE needs work: INFO: analyzing test_foreign_inherit.parent INFO: parent: scanned 1 of 1 pages, containing 1 live rows and 0 dead rows; 1 rows in sample, 1 estimated total rows INFO: analyzing test_foreign_inherit.parent inheritance tree WARNING: relcache reference leak: relation child not closed WARNING: relcache reference leak: relation tchild not closed WARNING: relcache reference leak: relation parent not closed Please arrange to omit the 'analyzing tablename inheritance tree' message, since this ANALYZE actually skipped that task. (The warnings obviously need a fix, too.) I do find it awkward that adding a foreign table to an inheritance tree will make autovacuum stop collecting statistics on that inheritance tree, but I can't think of a better policy. I think it would be better that this is handled in the same way as an inheritance tree that turns out to be a singe table that doesn't have any descendants in acquire_inherited_sample_rows(). That would still output the message as shown below, but I think that that would be more consistent with the existing code. The patch works so. (The warnings are also fixed.) INFO: analyzing public.parent INFO: parent: scanned 0 of 0 pages, containing 0 live rows and 0 dead rows; 0 rows in sample, 0 estimated total rows INFO: analyzing public.parent inheritance tree INFO: analyzing pg_catalog.pg_authid INFO: pg_authid: scanned 1 of 1 pages, containing 1 live rows and 0 dead rows; 1 rows in sample, 1 estimated total rows Today's ANALYZE VERBOSE messaging for former inheritance parents (tables with relhassubclass = true but no pg_inherits.inhparent links) is deceptive, and I welcome a fix to omit the spurious message. As defects go, this is quite minor. There's fundamentally no value in collecting inheritance tree statistics for such a parent, and no PostgreSQL command will do so. Your proposed behavior for inheritance parents having at least one foreign table child is more likely to mislead DBAs in practice. An inheritance tree genuinely exists, and a different ANALYZE command is quite capable of collecting statistics on that inheritance tree. Messaging should reflect the difference between ANALYZE invocations that do such work and ANALYZE invocations that skip it. I'm anticipating a bug report along these lines: I saw poor estimates involving a child foreign table, so I ran ANALYZE VERBOSE, which reported 'INFO: analyzing public.parent inheritance tree'. Estimates remained poor, so I scratched my head for awhile before noticing that pg_stats didn't report such statistics. I then ran ANALYZE VERBOSE parent, saw the same 'INFO: analyzing public.parent inheritance tree', but this time saw relevant rows in pg_stats. The message doesn't reflect the actual behavior. I'll sympathize with that complaint. It's a minor point overall, but the code impact is, I predict, small enough that we may as well get it right. A credible alternative is to emit a second message indicating that we skipped the inheritance tree statistics after all, and why we skipped them. -- 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] [GSoC2014] Patch ALTER TABLE ... SET LOGGED
On Thu, Aug 21, 2014 at 10:26 PM, Fabrízio de Royes Mello fabriziome...@gmail.com wrote: On Thu, Aug 21, 2014 at 8:04 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Andres Freund wrote: Have you looked at the correctness of the patch itself? Last time I'd looked it has fundamental correctness issues. I'd outlined a possible solution, but I haven't looked since. Yeah, Fabrizio had it passing the relpersistence down to make_new_heap, so the transient table is created with the right setting. AFAICS it's good now. I'm a bit uneasy about the way it changes indexes: it just updates pg_class for them just before invoking the reindex in finish_heap_swap. I think it's correct as it stands though; at least, the rewrite phase happens with the right setting, so that if there are constraints being checked and these invoke index scans, such accesses would not leave buffers with the wrong setting in shared_buffers. Ok. Another option would be to pass the new relpersistence down to finish_heap_swap, but that would be hugely complicated AFAICS. I think isn't so complicated to do it, but will this improve something ? Maybe I didn't understand it very well. IMHO it just complicate a simple thing. Here's the updated patch. Thanks Alvaro! I forgot to mention... I did again a lot of tests using different replication scenarios to make sure all is ok: - slaves async - slaves sync - cascade slaves 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
Re: [HACKERS] Removing dependency to wsock32.lib when compiling code on WIndows
On Thu, Aug 21, 2014 at 09:18:22AM -0400, Andrew Dunstan wrote: On 08/15/2014 11:00 PM, Noah Misch wrote: On Fri, Aug 15, 2014 at 12:49:36AM -0700, Michael Paquier wrote: Btw, how do you determine if MSVC is using HAVE_GETADDRINFO? Is it decided by the inclusion of getaddrinfo.c in @pgportfiles of Mkvdbuild.pm? src/include/pg_config.h.win32 dictates it, and we must keep @pgportfiles synchronized with the former's verdict. What's happening about this? Buildfarm animal jacana is consistently red because of this. If nobody plans to do the aforementioned analysis in the next 4-7 days, I suggest we adopt one of Michael's suggestions: force configure to reach its old conclusion about getaddrinfo() on Windows. Then the analysis can proceed on an unhurried schedule. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Are postgresql-9.4 binaries available on Windows XP?
Hi Dave and Andrew, I recently noticed the thread [BUGS] BUG #11039: installation fails when trying to install C++ redistributable . Unfortunately I have no XP machine at hand and can't test the installer by myself. Looking at the binaries in the package, they seem to be built using Visual Studio 2013. I'm suspicious if the binaries are available on Windows XP. If I recognize correctly, Visual Studio 2012 or later doesn't support Windows XP by default and Platform Toolset v120_xp (or v110_xp) must be specified so as to build binaries guaranteed to be avaiable on Windows XP. However MSBuildProject.pm seems to specify v120 (or v110) as the PlarformToolset property. Is it intentional? regards, Hiroshi Inoue -- 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] [GSoC2014] Patch ALTER TABLE ... SET LOGGED
Fabrízio de Royes Mello wrote: I forgot to mention... I did again a lot of tests using different replication scenarios to make sure all is ok: - slaves async - slaves sync - cascade slaves On v13 you mean? -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [GSoC2014] Patch ALTER TABLE ... SET LOGGED
Em sexta-feira, 22 de agosto de 2014, Alvaro Herrera alvhe...@2ndquadrant.com escreveu: Fabrízio de Royes Mello wrote: I forgot to mention... I did again a lot of tests using different replication scenarios to make sure all is ok: - slaves async - slaves sync - cascade slaves On v13 you mean? Exactly! Fabrízio Mello -- 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
Re: [HACKERS] inherit support for foreign tables
Noah Misch wrote: I'm anticipating a bug report along these lines: I saw poor estimates involving a child foreign table, so I ran ANALYZE VERBOSE, which reported 'INFO: analyzing public.parent inheritance tree'. Estimates remained poor, so I scratched my head for awhile before noticing that pg_stats didn't report such statistics. I then ran ANALYZE VERBOSE parent, saw the same 'INFO: analyzing public.parent inheritance tree', but this time saw relevant rows in pg_stats. The message doesn't reflect the actual behavior. I'll sympathize with that complaint. Agreed on that. A credible alternative is to emit a second message indicating that we skipped the inheritance tree statistics after all, and why we skipped them. That'd be similar to Xorg emitting messages such as [53.772] (II) intel(0): RandR 1.2 enabled, ignore the following RandR disabled message. [53.800] (--) RandR disabled I find this very poor. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] option -T in pg_basebackup doesn't work on windows
On Thu, Aug 21, 2014 at 3:44 PM, Amit Kapila amit.kapil...@gmail.com wrote: On Tue, Aug 19, 2014 at 9:51 AM, Amit Kapila amit.kapil...@gmail.com wrote: On Mon, Aug 18, 2014 at 7:50 PM, Heikki Linnakangas hlinnakan...@vmware.com wrote: Wouldn't it make a lot more sense to create it correctly in the first place? Looking at the code, I think it is very well possible to create it correctly in the first place without much extra work. I will send a patch if nobody sees any problem with this change. Attached patch implements the above suggested fix. I have removed the earlier code which was used to update the symlink path. Today morning, I realised that there is one problem with the patch I sent yesterday and the problem is that incase user has not given -T option, it will not be able to create the symlink for appropriate path. Attached patch fix this issue. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com pg_basebackup_relocate_tablespace_v4.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_receivexlog --status-interval add fsync feedback
Thank you for updating the patch. I reviewed the patch. First of all, I think that we should not append the above message to section of '-r' option. (Or these message might not be needed at all) Whether flush location in feedback message is valid, is not depend on '-r' option. If we use '-r' option and 'S' option (i.g., replication slot) then pg_receivexlog informs valid flush location to primary server at the same time as doing fsync. But, if we don't specify replication slot then the flush location in feedback message always invalid. So I think Fujii-san pointed out that sending of invalid flush location is not needed if pg_receivexlog does not use replication slot. Thanks for the review! I understand the attention message wasn't appropriate. To report the write location, even If you do not specify a replication slot. So the fix only appended messages. There was a description of the flush location section of '-S' option, but I intended to catch eye more and added a message. Is it better to make specification of the -S option indispensable? Regards, -- Furuya Osamu -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: Compute attr_needed for child relations (was Re: [HACKERS] inherit support for foreign tables)
On Thu, Aug 21, 2014 at 3:00 PM, Etsuro Fujita fujita.ets...@lab.ntt.co.jp wrote: (2014/08/21 13:21), Ashutosh Bapat wrote: On Wed, Aug 20, 2014 at 3:25 PM, Etsuro Fujita fujita.ets...@lab.ntt.co.jp mailto:fujita.ets...@lab.ntt.co.jp wrote: Hi Ashutish, I am sorry that I mistook your name's spelling. (2014/08/14 22:30), Ashutosh Bapat wrote: On Thu, Aug 14, 2014 at 10:05 AM, Etsuro Fujita fujita.ets...@lab.ntt.co.jp mailto:fujita.ets...@lab.ntt.co.jp mailto:fujita.ets...@lab.ntt.__co.jp mailto:fujita.ets...@lab.ntt.co.jp wrote: (2014/08/08 18:51), Etsuro Fujita wrote: (2014/06/30 22:48), Tom Lane wrote: I wonder whether it isn't time to change that. It was coded like that originally only because calculating the values would've been a waste of cycles at the time. But this is at least the third place where it'd be useful to have attr_needed for child rels. There was a problem with the previous patch, which will be described below. Attached is the updated version of the patch addressing that. Here are some more comments attr_needed also has the attributes used in the restriction clauses as seen in distribute_qual_to_rels(), so, it looks unnecessary to call pull_varattnos() on the clauses in baserestrictinfo in functions check_selective_binary___conversion(), remove_unused_subquery___outputs(), check_index_only(). IIUC, I think it's *necessary* to do that in those functions since the attributes used in the restriction clauses in baserestrictinfo are not added to attr_needed due the following code in distribute_qual_to_rels. That's right. Thanks for pointing that out. Although in case of RTE_RELATION, the 0th entry in attr_needed corresponds to FirstLowInvalidHeapAttributeNu__mber + 1, it's always safer to use it is RelOptInfo::min_attr, in case get_relation_info() wants to change assumption or somewhere down the line some other part of code wants to change attr_needed information. It may be unlikely, that it would change, but it will be better to stick to comments in RelOptInfo 443 AttrNumber min_attr; /* smallest attrno of rel (often 0) */ 444 AttrNumber max_attr; /* largest attrno of rel */ 445 Relids *attr_needed;/* array indexed [min_attr .. max_attr] */ Good point! Attached is the revised version of the patch. If the patch is not in the commit-fest, can you please add it there? I've already done that: https://commitfest.postgresql.org/action/patch_view?id=1529 From my side, the review is done, it should be marked ready for committer, unless somebody else wants to review. Many thanks! Thanks. Since, I haven't seen anybody else commenting here and I do not have any further comments to make, I have marked it as ready for committer. Best regards, Etsuro Fujita -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company
Re: [HACKERS] Proposal to add a QNX 6.5 port to PostgreSQL
On Thu, Aug 21, 2014 at 01:33:38AM +0200, Andres Freund wrote: On 2014-07-25 18:29:53 -0400, Tom Lane wrote: * QNX lacks sigaction SA_RESTART: I modified src/include/port.h to define macros to retry system calls upon EINTR (open,read,write,...) when compiled on QNX That's pretty scary too. For one thing, such macros would affect every call site whether it's running with SA_RESTART or not. Do you really need it? It looks to me like we just turn off HAVE_POSIX_SIGNALS if you don't have SA_RESTART. Maybe that code has bit-rotted by now, but it did work at one time. I have pretty much no trust that we're maintaining !HAVE_POSIX_SIGNAL. And none that we have that capability of doing so. I seriously doubt there's any !HAVE_POSIX_SIGNAL animals and 873ab97219caabeb2f7b390268a4fe01e2b7518c makes it pretty darn unlikely that we have much chance of finding such mistakes during development. I bet it's fine for its intended target, namely BSD-style signal() in which SA_RESTART-like behavior is implicit. See the src/port/pqsignal.c header comment. PostgreSQL has no support for V7-style/QNX-style signal(). -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
