Re: [HACKERS] Will Index-only-scan be in 9.2
On Wed, 2011-10-12 at 02:08 +0200, hans wulf wrote: > I was wondering if the index-only-scan will be available in 9.2? This > is not having to visit the real data to answer a query, if all the > information is available in the index. I think this will be a mayor > step in overtaking the big O in the g-spot. > http://rhaas.blogspot.com/2011/10/index-only-scans-weve-got-em.html Regards, -- Devrim GÜNDÜZ Principal Systems Engineer @ EnterpriseDB: http://www.enterprisedb.com PostgreSQL Danışmanı/Consultant, Red Hat Certified Engineer Community: devrim~PostgreSQL.org, devrim.gunduz~linux.org.tr http://www.gunduz.org Twitter: http://twitter.com/devrimgunduz signature.asc Description: This is a digitally signed message part
[HACKERS] Will Index-only-scan be in 9.2
I was wondering if the index-only-scan will be available in 9.2? This is not having to visit the real data to answer a query, if all the information is available in the index. I think this will be a mayor step in overtaking the big O in the g-spot. There seams to already be some patch for this problem, but I don't know if you are considering this for the next release? Why is the enthusiasm so low on this feature? I would presume this is a mayor performance boost, not having to run through millons of stupid pages if all the info is already in the index. -- NEU: FreePhone - 0ct/min Handyspartarif mit Geld-zurück-Garantie! Jetzt informieren: http://www.gmx.net/de/go/freephone -- 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] pl/perl example in the doc no longer works in 9.1
On Thu, Oct 13, 2011 at 16:05, Tom Lane wrote: > > Applied with some further hacking of my own to clean up memory leaks > and grotty coding. Thanks! BTW after seeing it I agree passing in fcinfo (and the other fixes) to plperl_sv_to_datum() is better. -- 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] WALInsertLock tuning
I assume this was addressed with this commit: commit 465883b0a2b4236ba6b31b648a9eabef3b7cdddb Author: Simon Riggs Date: Tue Jun 28 22:58:17 2011 +0100 Introduce compact WAL record for the common case of commit (non-DDL). XLOG_XACT_COMMIT_COMPACT leaves out invalidation messages and relfilenodes, saving considerable space for the vast majority of transaction commits. XLOG_XACT_COMMIT keeps same definition as XLOG_PAGE_MAGIC 0xD067 and earlier. Leonardo Francalanci and Simon Riggs --- Simon Riggs wrote: > On Mon, Jun 6, 2011 at 11:25 PM, Robert Haas wrote: > > > As to the question of whether it's safe, I think I'd agree that the > > chances of this backfiring are pretty remote. ?I think that with the > > zeroing they are exactly zero, because (now that we start XLOG > > positions at 0/1) there is now way that xl_prev = {0, 0} can ever be > > valid. ?Without the zeroing, well, it's remotely possible that xl_prev > > could happen to appear valid and that xl_crc could happen to match... > > but the chances are presumably quite remote. ?Just the chances of the > > CRC matching should be around 2^-32. ?The chances of an xl_prev match > > are harder to predict, because the matching values for CRCs should be > > pretty much uniformly distributed, while xl_prev isn't random. ?But > > even given that the chance of a match is should be very small, so in > > practice there is likely no harm. > > And if such a thing did actually happen we would also need to have an > accidentally correct value of all of the rest of the header values. > And even if we did we would apply at most one junk WAL record. Then we > are onto the next WAL record where we would need have to the same luck > all over again. > > The probability of these occurrences is well below the acceptable > threshold for other problems occurring. > > > It strikes me, though, that we > > could probably get nearly all of the benefit of this patch by being > > willing to zero the first sizeof(XLogRecord) bytes following a record, > > but not the rest of the buffer. ?That would pretty much wipe out any > > chance of an xl_prev match, I think, and would likely still get nearly > > all of the performance benefit. > > Which adds something onto the path of every XlogInsert(), rather than > once per page, so I'm a little hesitant to agree. > > If we did that, we would only need to write out an additional 12 bytes > per WAL record, not the full sizeof(XLogRecord). > > But even so, I think its wasted effort. > > Measuring the benefit of a performance patch is normal, but I'm not > proposing this as a risk trade-off. It's just a straight removal of > multiple cycles from a hot code path. The exact benefit will depend > upon whether the WALInsertLock is the hot lock, which it likely will > be when other patches are applied. > > -- > ?Simon Riggs?? 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 -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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: AuthenticationMD5 protocol documentation clarification
Heikki Linnakangas wrote: > On 06.06.2011 16:58, Robert Haas wrote: > > On Sun, Jun 5, 2011 at 11:26 AM, Cyan Ogilvie > > wrote: > >> This is my first patch, so I hope I've got the process right for submitting > >> patches. > > > > You're doing great. I suspect we do want to either (1) reword what > > you've done in English, rather than writing it as code, or at least > > (2) add some SGML markup to the code. Our next CommitFest starts in > > just over a week, so you should receive some more specific feedback > > pretty soon. > > That is quite complicated to explain in plain English, so some sort of > pseudo-code is probably a good idea. I would recommend not to formulate > it as a SQL expression, though. It makes you think you could execute it > from psql or something. Even if you know that's not how to do it, it > feels confusing. Maybe something like: > > md5 hex_encode(md5(hex_encode(md5(password username) > salt) > > with some extra markup to make it look pretty. I have applied the attached doc patch to document this. Thanks for the report --- it was something we certainly needed to document. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml new file mode 100644 index 19c9686..4fda518 *** a/doc/src/sgml/protocol.sgml --- b/doc/src/sgml/protocol.sgml *** *** 293,302 The frontend must now send a PasswordMessage containing the ! password encrypted via MD5, using the 4-character salt ! specified in the AuthenticationMD5Password message. If ! this is the correct password, the server responds with an ! AuthenticationOk, otherwise it responds with an ErrorResponse. --- 293,307 The frontend must now send a PasswordMessage containing the ! password (with username) encrypted via MD5, then encrypted ! again using the 4-byte random salt specified in the ! AuthenticationMD5Password message. If this is the correct ! password, the server responds with an AuthenticationOk, ! otherwise it responds with an ErrorResponse. The actual ! PasswordMessage can be computed in SQL as concat('md5', ! md5(concat(md5(concat(password, username)), random-salt))). ! (Keep in mind the md5() function returns its ! result as a hex string.) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch for new feature: Buffer Cache Hibernation
Should this be marked as TODO? --- Mitsuru IWASAKI wrote: > Hi, > > > On 05/07/2011 03:32 AM, Mitsuru IWASAKI wrote: > > > For 1, I've just finish my work. The latest patch is available at: > > > http://people.freebsd.org/~iwasaki/postgres/buffer-cache-hibernation-postgresql-20110507.patch > > > > > > > Reminder here--we can't accept code based on it being published to a web > > page. You'll need to e-mail it to the pgsql-hackers mailing list to be > > considered for the next PostgreSQL CommitFest, which is starting in a > > few weeks. Code submitted to the mailing list is considered a release > > of it to the project under the PostgreSQL license, which we can't just > > assume for things when given only a URL to them. > > Sorry about that, but I had enough time to revise my patches this week-end. > I attached the patches in this mail, and will update CommitFest page soon. > > > Also, you suggested you were out of time to work on this. If that's the > > case, we'd like to know that so we don't keep cc'ing you about things in > > expectation of an answer. Someone else may pick this up as a project to > > continue working on. But it's going to need a fair amount of revision > > before it matches what people want here, and I'm not sure how much of > > what you've written is going to end up in any commit that may happen > > from this idea. > > It seems that I don't have enough time to complete this work. > You don't need to keep cc'ing me, and I'm very happy if postgres to be > the first DBMS which support buffer cache hibernation feature. > > Thanks! > > > diff --git src/backend/access/transam/xlog.c src/backend/access/transam/xlog.c > index b0e4c41..7a3a207 100644 > --- src/backend/access/transam/xlog.c > +++ src/backend/access/transam/xlog.c > @@ -4834,6 +4834,19 @@ ReadControlFile(void) > #endif > } > > +bool > +GetControlFile(ControlFileData *controlFile) > +{ > + if (ControlFile == NULL) > + { > + return false; > + } > + > + memcpy(controlFile, ControlFile, sizeof(ControlFileData)); > + > + return true; > +} > + > void > UpdateControlFile(void) > { > diff --git src/backend/bootstrap/bootstrap.c src/backend/bootstrap/bootstrap.c > index fc093cc..7ecf6bb 100644 > --- src/backend/bootstrap/bootstrap.c > +++ src/backend/bootstrap/bootstrap.c > @@ -360,6 +360,15 @@ AuxiliaryProcessMain(int argc, char *argv[]) > BaseInit(); > > /* > + * Only StartupProcess can call ResumeBufferCacheHibernation() after > + * InitFileAccess() and smgrinit(). > + */ > + if (auxType == StartupProcess && BufferCacheHibernationLevel > 0) > + { > + ResumeBufferCacheHibernation(); > + } > + > + /* >* When we are an auxiliary process, we aren't going to do the full >* InitPostgres pushups, but there are a couple of things that need to > get >* lit up even in an auxiliary process. > diff --git src/backend/storage/buffer/buf_init.c > src/backend/storage/buffer/buf_init.c > index dadb49d..52eb51a 100644 > --- src/backend/storage/buffer/buf_init.c > +++ src/backend/storage/buffer/buf_init.c > @@ -127,6 +127,14 @@ InitBufferPool(void) > > /* Init other shared buffer-management stuff */ > StrategyInitialize(!foundDescs); > + > + if (BufferCacheHibernationLevel > 0) > + { > + > ResisterBufferCacheHibernation(BUFFER_CACHE_HIBERNATION_TYPE_DESCRIPTORS, > + (char *)BufferDescriptors, sizeof(BufferDesc), > NBuffers); > + > ResisterBufferCacheHibernation(BUFFER_CACHE_HIBERNATION_TYPE_BLOCKS, > + (char *)BufferBlocks, BLCKSZ, NBuffers); > + } > } > > /* > diff --git src/backend/storage/buffer/bufmgr.c > src/backend/storage/buffer/bufmgr.c > index f96685d..dba8ebf 100644 > --- src/backend/storage/buffer/bufmgr.c > +++ src/backend/storage/buffer/bufmgr.c > @@ -31,6 +31,7 @@ > #include "postgres.h" > > #include > +#include > #include > > #include "catalog/catalog.h" > @@ -61,6 +62,13 @@ > #define BUF_WRITTEN 0x01 > #define BUF_REUSABLE 0x02 > > +/* > + * Buffer Cache Hibernation stuff. > + */ > +/* enable this to debug buffer cache hibernation. */ > +#if 0 > +#define DEBUG_BUFFER_CACHE_HIBERNATION > +#endif > > /* GUC variables */ > bool zero_damaged_pages = false; > @@ -765,6 +773,16 @@ BufferAlloc(SMgrRelation smgr, char relpersistence, > ForkNumber forkNum, > } > } > > +#ifdef DEBUG_BUFFER_CACHE_HIBERNATION > + elog(DEBUG5, > + "alloc > [%d]\t%03x,%d,%d,%d,%d\t%08x,%d,%d,%d,%d,%d", > + buf->buf_id, buf->flags, > buf->usage_count, buf->refcount, > + buf->wait_backend_pid, buf->freeNext, > +
Re: [HACKERS] Remove support for 'userlocks'?
Josh Berkus wrote: > On 6/3/11 11:01 AM, Bruce Momjian wrote: > > According to our documentation, 'userlocks' were removed in PG 8.2: > > > > > > http://developer.postgresql.org/pgdocs/postgres/runtime-config-developer.html > > I take it this doesn't trace advisory locks, and trace_locks does? > > If so, then by all means remove the code. I highly doubt anyone is > using that option anyway ... The attached, applied patch removes all "traces" of trace_userlocks. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml new file mode 100644 index d3a8b26..fa2dcf3 *** a/doc/src/sgml/config.sgml --- b/doc/src/sgml/config.sgml *** LOG: CleanUpLock: deleting: lock(0xb7ac *** 6193,6220 - trace_userlocks (boolean) - -trace_userlocks configuration parameter - - - - If on, emit information about user lock usage. Output is the same - as for trace_locks, only for user locks. - - - User locks were removed as of PostgreSQL version 8.2. This option - currently has no effect. - - - This parameter is only available if the LOCK_DEBUG - macro was defined when PostgreSQL was - compiled. - - - - - trace_lock_oidmin (integer) trace_lock_oidmin configuration parameter --- 6193,6198 diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c new file mode 100644 index 905502f..ed8344f *** a/src/backend/storage/lmgr/lock.c --- b/src/backend/storage/lmgr/lock.c *** static const LockMethodData user_lockmet *** 213,224 AccessExclusiveLock, /* highest valid lock mode number */ true, LockConflicts, ! lock_mode_names, ! #ifdef LOCK_DEBUG ! &Trace_userlocks ! #else ! &Dummy_trace ! #endif }; /* --- 213,219 AccessExclusiveLock, /* highest valid lock mode number */ true, LockConflicts, ! lock_mode_names }; /* *** static ResourceOwner awaitedOwner; *** 276,282 int Trace_lock_oidmin = FirstNormalObjectId; bool Trace_locks = false; - bool Trace_userlocks = false; int Trace_lock_table = 0; bool Debug_deadlocks = false; --- 271,276 diff --git a/src/backend/utils/misc/check_guc b/src/backend/utils/misc/check_guc new file mode 100755 index 293fb03..91c1c58 *** a/src/backend/utils/misc/check_guc --- b/src/backend/utils/misc/check_guc *** INTENTIONALLY_NOT_INCLUDED="autocommit d *** 20,26 is_superuser lc_collate lc_ctype lc_messages lc_monetary lc_numeric lc_time \ pre_auth_delay role seed server_encoding server_version server_version_int \ session_authorization trace_lock_oidmin trace_lock_table trace_locks trace_lwlocks \ ! trace_notify trace_userlocks transaction_isolation transaction_read_only \ zero_damaged_pages" ### What options are listed in postgresql.conf.sample, but don't appear --- 20,26 is_superuser lc_collate lc_ctype lc_messages lc_monetary lc_numeric lc_time \ pre_auth_delay role seed server_encoding server_version server_version_int \ session_authorization trace_lock_oidmin trace_lock_table trace_locks trace_lwlocks \ ! trace_notify transaction_isolation transaction_read_only \ zero_damaged_pages" ### What options are listed in postgresql.conf.sample, but don't appear diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c new file mode 100644 index 106096f..f1d35a9 *** a/src/backend/utils/misc/guc.c --- b/src/backend/utils/misc/guc.c *** static struct config_bool ConfigureNames *** 1059,1074 NULL, NULL, NULL }, { - {"trace_userlocks", PGC_SUSET, DEVELOPER_OPTIONS, - gettext_noop("No description available."), - NULL, - GUC_NOT_IN_SAMPLE - }, - &Trace_userlocks, - false, - NULL, NULL, NULL - }, - { {"trace_lwlocks", PGC_SUSET, DEVELOPER_OPTIONS, gettext_noop("No description available."), NULL, --- 1059,1064 diff --git a/src/include/storage/lock.h b/src/include/storage/lock.h new file mode 100644 index e106ad5..bc746a3 *** a/src/include/storage/lock.h --- b/src/include/storage/lock.h *** extern int max_locks_per_xact; *** 34,40 #ifdef LOCK_DEBUG extern int Trace_lock_oidmin; extern bool Trace_locks; - extern bool Trace_userlocks; extern int Trace_lock_table; extern bool Debug_deadlocks; #endif /* LOCK_DEBUG */ --- 34,39 -- 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] Additional supplied modules
Josh Berkus writes: > On 10/13/11 6:32 AM, Thom Brown wrote: >> Could we somehow categorise these, and also do something to clarify >> that SPI is a collection of extensions rather than an extension >> itself? > Alternately we should clean up SPI and break it out into its separate > extensions. I don't feel a need to do that in the source tree, but maybe flattening the documentation would be worthwhile. 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] pl/perl example in the doc no longer works in 9.1
Alex Hunsaker writes: > This gets rid of of most of the if/else chain and the has_retval crap > in plperl_handl_func(). Instead we let plperl_sv_to_datum() do most of > the lifting. It also now handles VOIDOID and checks that the request > result oid can be converted from the perl structure. For example if > you passed in a hashref with a result oid that was not an rowtype it > will error out with "PL/Perl cannot convert hash to non rowtype %s". > Arrays behave similarly. Applied with some further hacking of my own to clean up memory leaks and grotty coding. 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] pl/perl example in the doc no longer works in 9.1
On Oct 13, 2011, at 9:02 PM, Tom Lane wrote: > Alex Hunsaker writes: >> On Wed, Oct 12, 2011 at 15:33, Alex Hunsaker wrote: >>> On Wed, Oct 12, 2011 at 15:00, Tom Lane wrote: The core of the problem seems to be that if SvROK(sv) then the code assumes that it must be intended to convert that to an array or composite, no matter whether the declared result type of the function is compatible with such a thing. > >> PFA my attempt at a fix. > >> This gets rid of of most of the if/else chain and the has_retval crap >> in plperl_handl_func(). Instead we let plperl_sv_to_datum() do most of >> the lifting. It also now handles VOIDOID and checks that the request >> result oid can be converted from the perl structure. For example if >> you passed in a hashref with a result oid that was not an rowtype it >> will error out with "PL/Perl cannot convert hash to non rowtype %s". >> Arrays behave similarly. > > I'm working through this patch now. Does anyone object to having the > array-to-non-array-result-type and hash-to-non-rowtype-result-type cases > throw errors, rather than returning the rather useless ARRAY(...) and > HASH(...) strings as pre-9.1 did? No objections here. -- Alexey Klyukinhttp://www.commandprompt.com The PostgreSQL Company – Command Prompt, Inc. -- 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] Additional supplied modules
On 13 October 2011 19:46, Dimitri Fontaine wrote: > Josh Berkus writes: >> On 10/13/11 6:32 AM, Thom Brown wrote: >>> Could we somehow categorise these, and also do something to clarify >>> that SPI is a collection of extensions rather than an extension >>> itself? >> >> Alternately we should clean up SPI and break it out into its separate >> extensions. > > Additionaly we need to pick some and bless them as in-core extensions > (installed by default, you still need to CREATE EXTENSION to benefit > from them) and move the others into either an "example" section or an > "additional" section where production ready goodies are to be found but > just were not selected as in-core extensions for whatever reasons > (export laws in the case of pgcrypto, some non portable dependencies for > other choices, etc). > > We might even have some more categories into, such as "debug", > "profile", "data type", "compat" and all. Maybe a new field with that > kind of classification in the control file would be good (it would be > found in the catalogs too). An extra bit of confusion comes with installing languages as extensions. These aren't considered to be one of the additional supplied modules, so not all items which require installing via CREATE EXTENSION are shown alongside others. And the CREATE EXTENSION page doesn't mention function languages at all. -- Thom Brown Twitter: @darkixion IRC (freenode): dark_ixion Registered Linux user: #516935 EnterpriseDB UK: 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] Additional supplied modules
Josh Berkus writes: > On 10/13/11 6:32 AM, Thom Brown wrote: >> Could we somehow categorise these, and also do something to clarify >> that SPI is a collection of extensions rather than an extension >> itself? > > Alternately we should clean up SPI and break it out into its separate > extensions. Additionaly we need to pick some and bless them as in-core extensions (installed by default, you still need to CREATE EXTENSION to benefit from them) and move the others into either an "example" section or an "additional" section where production ready goodies are to be found but just were not selected as in-core extensions for whatever reasons (export laws in the case of pgcrypto, some non portable dependencies for other choices, etc). We might even have some more categories into, such as "debug", "profile", "data type", "compat" and all. Maybe a new field with that kind of classification in the control file would be good (it would be found in the catalogs too). Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] about EDITOR_LINENUMBER_SWITCH
Alvaro Herrera wrote: > Excerpts from Tom Lane's message of mi?? may 25 16:07:55 -0400 2011: > > Alvaro Herrera writes: > > > Excerpts from Tom Lane's message of mar may 24 17:11:17 -0400 2011: > > >> Right. It would also increase the cognitive load on the user to have > > >> to remember the command-line go-to-line-number switch for his editor. > > >> So I don't particularly want to redesign this feature. However, I can > > >> see the possible value of letting EDITOR_LINENUMBER_SWITCH be set from > > >> the same place that you set EDITOR, which would suggest that we allow > > >> the value to come from an environment variable. I'm not sure whether > > >> there is merit in allowing both that source and ~/.psqlrc, though > > >> possibly for Windows users it might be easier if ~/.psqlrc worked. > > > > > If we're going to increase the number of options in .psqlrc that do not > > > work with older psql versions, can I please have .psqlrc-MAJORVERSION or > > > some such? Having 8.3's psql complain all the time because it doesn't > > > understand "linestyle" is annoying. > > > > 1. I thought we already did have that. > > Oh, true, we have that, though it's not very usable because you have to > rename the file from .psqlrc-9.0.3 to .psqlrc-9.0.4 when you upgrade, > which is kinda silly. True. We don't add configuration changes in minor versions so having minor-version granularity makes no sense. The attached patch changes this to use the _major_ version number for psql rc files. Does this have to be backward-compatible? Should I check for minor and major matches? That is going to be confusing to document. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml new file mode 100644 index 662eab7..1ea728b *** a/doc/src/sgml/ref/psql-ref.sgml --- b/doc/src/sgml/ref/psql-ref.sgml *** PSQL_EDITOR_LINENUMBER_ARG='--line ' *** 3332,3338 Both the system-wide psqlrc file and the user's ~/.psqlrc file can be made version-specific by appending a dash and the PostgreSQL ! release number, for example ~/.psqlrc-&version;. A matching version-specific file will be read in preference to a non-version-specific file. --- 3332,3338 Both the system-wide psqlrc file and the user's ~/.psqlrc file can be made version-specific by appending a dash and the PostgreSQL ! major release number, for example ~/.psqlrc-9.2. A matching version-specific file will be read in preference to a non-version-specific file. diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c new file mode 100644 index 3c17eec..9670b7e *** a/src/bin/psql/startup.c --- b/src/bin/psql/startup.c *** process_psqlrc_file(char *filename) *** 600,607 #define R_OK 4 #endif ! psqlrc = pg_malloc(strlen(filename) + 1 + strlen(PG_VERSION) + 1); ! sprintf(psqlrc, "%s-%s", filename, PG_VERSION); if (access(psqlrc, R_OK) == 0) (void) process_file(psqlrc, false, false); --- 600,607 #define R_OK 4 #endif ! psqlrc = pg_malloc(strlen(filename) + 1 + strlen(PG_MAJORVERSION) + 1); ! sprintf(psqlrc, "%s-%s", filename, PG_MAJORVERSION); if (access(psqlrc, R_OK) == 0) (void) process_file(psqlrc, false, false); -- 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] pl/perl example in the doc no longer works in 9.1
On Oct 13, 2011, at 11:25 AM, Tom Lane wrote: >> Certainly not in 9.2, no. Not sure about 9.1, though. > > Well, right now 9.1 is returning some rather random results. If we > don't change it, someone might claim that later releases ought to be > compatible with that ... Okay then, works for me. D -- 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] pl/perl example in the doc no longer works in 9.1
"David E. Wheeler" writes: > On Oct 13, 2011, at 11:02 AM, Tom Lane wrote: >> I'm working through this patch now. Does anyone object to having the >> array-to-non-array-result-type and hash-to-non-rowtype-result-type cases >> throw errors, rather than returning the rather useless ARRAY(...) and >> HASH(...) strings as pre-9.1 did? > Certainly not in 9.2, no. Not sure about 9.1, though. Well, right now 9.1 is returning some rather random results. If we don't change it, someone might claim that later releases ought to be compatible with 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] ts_rank
Oleg Bartunov wrote: > I'm sorry, my plane to Nepal is waiting me :) I'll be back in the > midst of November. In short, ts_rank is based only on frequencies of lexems > and doesn't count distance between query lexems. Also, it supports only > primitive queries. Thanks. Attached doc patch applied to head and 9.1.X. --- > > Oleg > On Wed, 12 Oct 2011, Bruce Momjian wrote: > > > Bruce Momjian wrote: > >> Mark wrote: > There's some potentially useful information here: > http://www.postgresql.org/docs/9.0/interactive/textsearch-controls.html#TEXTSEARCH-RANKING > >>> > >>> Thanks for reply. I was reading the documentation of PostgreSQL, but there > >>> it is not written the name of the used methods. Everywhere there is > >>> written, > >>> that ts_rank use standard ranking function. But it is difficult to say > >>> which > >>> is the standard function. > >>> Somewhere I found that it is maybe based on Vector space model and it > >>> seems > >>> to be truth, because in the code of tsrank.c is counted the frequency of > >>> words, but I am not sure of that :-( > >> > >> Oleg, Teodor, can you give me a description of how ts_rank decided how > >> to rank items? Thanks. > > > > Any news on this question? > > > > > > Regards, > Oleg > _ > Oleg Bartunov, Research Scientist, Head of AstroNet (www.astronet.ru), > Sternberg Astronomical Institute, Moscow University, Russia > Internet: o...@sai.msu.su, http://www.sai.msu.su/~megera/ > phone: +007(495)939-16-83, +007(495)939-23-83 -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + diff --git a/doc/src/sgml/textsearch.sgml b/doc/src/sgml/textsearch.sgml new file mode 100644 index ef228e3..46db103 *** a/doc/src/sgml/textsearch.sgml --- b/doc/src/sgml/textsearch.sgml *** ts_rank( http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pl/perl example in the doc no longer works in 9.1
On Oct 13, 2011, at 11:02 AM, Tom Lane wrote: > I'm working through this patch now. Does anyone object to having the > array-to-non-array-result-type and hash-to-non-rowtype-result-type cases > throw errors, rather than returning the rather useless ARRAY(...) and > HASH(...) strings as pre-9.1 did? Certainly not in 9.2, no. Not sure about 9.1, though. Best, David -- 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] Additional supplied modules
On 10/13/11 6:32 AM, Thom Brown wrote: > Could we somehow categorise these, and also do something to clarify > that SPI is a collection of extensions rather than an extension > itself? Alternately we should clean up SPI and break it out into its separate extensions. -- 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] pl/perl example in the doc no longer works in 9.1
Alex Hunsaker writes: > On Wed, Oct 12, 2011 at 15:33, Alex Hunsaker wrote: >> On Wed, Oct 12, 2011 at 15:00, Tom Lane wrote: >>> The core of the problem seems to be that if SvROK(sv) then >>> the code assumes that it must be intended to convert that to an array or >>> composite, no matter whether the declared result type of the function is >>> compatible with such a thing. > PFA my attempt at a fix. > This gets rid of of most of the if/else chain and the has_retval crap > in plperl_handl_func(). Instead we let plperl_sv_to_datum() do most of > the lifting. It also now handles VOIDOID and checks that the request > result oid can be converted from the perl structure. For example if > you passed in a hashref with a result oid that was not an rowtype it > will error out with "PL/Perl cannot convert hash to non rowtype %s". > Arrays behave similarly. I'm working through this patch now. Does anyone object to having the array-to-non-array-result-type and hash-to-non-rowtype-result-type cases throw errors, rather than returning the rather useless ARRAY(...) and HASH(...) strings as pre-9.1 did? 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] [BUGS] BUG #6034: pg_upgrade fails when it should not.
Tom Lane wrote: > Florian Pflug writes: > > On Jun1, 2011, at 20:28 , Peter Eisentraut wrote: > >> Well, initdb still succeeds if you give it an invalid locale name. It > >> warns, but that can easily be missed if initdb is hidden behind a few > >> other layers. If you then run pg_upgrade, you get a hosed instance. > > > Whats the rational behind that behaviour? Wouldn't it be more user-friendly > > if initdb failed outright? It'd also be consistent with CREATE DATABASE... > > I think we were being conservative about whether initdb would get it > right. Might be time to stiffen our spines a bit, now that the logic > has been through a few release cycles. Do we want to revisit this? -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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] Dumping roles improvements?
Bruce Momjian writes: > We are not writing this software for you. Please submit a clear > proposal. I am sure you have 10k customers who want this. :-| I think I did (write this software). https://github.com/dimitri/pg_staging http://tapoueh.org/blog/2011/03/29-towards-pg_staging-10.html Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] Dumping roles improvements?
Josh Berkus writes: > (1) I cannot produce a single file in custom dump format which includes > both a single database and all of the roles I need to build that database. I would see addressing this with the proposal to have pg_dumpall able to issue an archive of -Fc dumps, that pg_restore would know how to handle. Then we could have some option to list or exclude databases you want in this pg_dumpall -Fc format. It's been said that extending custom format to handle several databases would be complex, but I don't think it's necessary. A simple archive, tar formatted for example, containing the custom format file of all selected databases, would be user friendly enough. All the more if you add a -j option to control how many databases you dump at the same time. The main drawback is that you need to prepare a directory with the globals.sql file and a custom format file per database, and only when all of those are finished can you pack them into the tar format. Again, good enough for me. > (2) I cannot dump a set of roles without md5 passwords. > > Both of these are things I need to support dev/stage/testing integration > at multiple sites. I don't handle that in pg_staging, but the other features you need are all implemented into that tool. It knows how to fetch the per cluster settings then init your staging cluster with that and only then restore your database (with a pgbouncer layer in between so that you can have more than one available in parallel and an easy switch from one to the other, like db -> db_20111013 rather than db_20111012). https://github.com/dimitri/pg_staging Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Change 'pg_ctl: no server running' Exit Status to 3
Peter Eisentraut wrote: > On m?n, 2011-05-23 at 20:50 -0400, Aaron W. Swenson wrote: > > According to Linux Standard Base Core Specification 3.1 [1], the exit > > status should be '3' when the server isn't running. > > > > I've attached a very simple patch that resolves this cosmetic issue, > > which applies to all branches. > > If we're going to make the exit status meaningful, it should probably be > added to the documentation. Patch attached and applied, with docs. I also cleaned up some C comments, which are not in the attached patch. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + 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 07836e7..7dc170e *** a/doc/src/sgml/ref/pg_ctl-ref.sgml --- b/doc/src/sgml/ref/pg_ctl-ref.sgml *** PostgreSQL documentation *** 205,211 status mode checks whether a server is running in the specified data directory. If it is, the PID and the command line options that were used to invoke it are !displayed. --- 205,212 status mode checks whether a server is running in the specified data directory. If it is, the PID and the command line options that were used to invoke it are !displayed. If the server is not running, the process returns an !exit status of 3. diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c new file mode 100644 index c9007ed..8e9d2ce *** a/src/bin/pg_ctl/pg_ctl.c --- b/src/bin/pg_ctl/pg_ctl.c *** do_status(void) *** 1186,1192 } } printf(_("%s: no server running\n"), progname); ! exit(1); } --- 1188,1198 } } printf(_("%s: no server running\n"), progname); ! /* ! * The Linux Standard Base Core Specification 3.1 says this should return '3' ! * http://refspecs.freestandards.org/LSB_3.1.1/LSB-Core-generic/LSB-Core-generic/iniscrptact.html ! */ ! exit(3); } -- 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] [bug] relcache leaks in get_object_address
The attached patch fixes this problem. Unfortunately, we have no code that invokes get_object_address() with missing_ok = true now, so please apply a couple of patches to rework DROP statement of mine. DROP TRIGGER no_such_trigger ON existing_table; shall cause a relcache reference leaks, without this patch. Thanks, 2011/10/13 Kohei KaiGai : > I noticed a problem of get_object_address() with missing_ok = true. > > When we try to solve the name of nonexistent rule/trigger/constraint on > a particular existing table, get_object_address_relobject() opens the > relation, but address.objectId = InvalidOid shall be set. > > I think it should be closed if the queried object was missing, although > existing code does not invoke get_object_address() with missing_ok = true. > > Thanks, > -- > KaiGai Kohei > -- KaiGai Kohei pgsql-v9.2-fix-relcache-leaks-in-get_object_address_relobject.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] Overhead cost of Serializable Snapshot Isolation
On Wed, Oct 12, 2011 at 10:50:13AM +0300, Peter Eisentraut wrote: > Actually, I'm currently personally more concerned about the breakage we > introduce in minor releases. We'd need to solve that problem before we > can even begin to think about dealing with the major release issue. +1 This bit me the other day. -- Greg Sabino Mullane g...@endpoint.com End Point Corporation PGP Key: 0x14964AC8 pgprM9aFgot2o.pgp Description: PGP signature
[HACKERS] [bug] relcache leaks in get_object_address
I noticed a problem of get_object_address() with missing_ok = true. When we try to solve the name of nonexistent rule/trigger/constraint on a particular existing table, get_object_address_relobject() opens the relation, but address.objectId = InvalidOid shall be set. I think it should be closed if the queried object was missing, although existing code does not invoke get_object_address() with missing_ok = true. Thanks, -- KaiGai Kohei -- 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] pl/perl example in the doc no longer works in 9.1
On Oct 13, 2011, at 7:09 AM, Alex Hunsaker wrote: > On Wed, Oct 12, 2011 at 15:33, Alex Hunsaker wrote: >> On Wed, Oct 12, 2011 at 15:00, Tom Lane wrote: > >>> The core of the problem seems to be that if SvROK(sv) then >>> the code assumes that it must be intended to convert that to an array or >>> composite, no matter whether the declared result type of the function is >>> compatible with such a thing. >> >> Hrm, well 9.0 and below did not get this "right" either: >> create or replace function test_hash() returns text as $$ return >> {'a'=>1}; $$ language plperl; >> select test_array(); >> test_array >> --- >> ARRAY(0x7fd92384dcb8) >> (1 row) >> >> create or replace function test_hash() returns text as $$ return >> {'a'=>1}; $$ language plperl; >> select test_hash(); >> test_hash >> -- >> HASH(0x7fd92387f848) >> (1 row) >> > >> Given the output above (both pre 9.1 and post) it seems unless the >> type is a set or composite we should throw an error. Maybe "PL/Perl >> function returning type %s must not return a reference" ? >> >>> It would be more appropriate to drive the >>> cases off the nature of the function result type, perhaps. >> >> Ill see if I can cook up something that's not too invasive. > > PFA my attempt at a fix. > > This gets rid of of most of the if/else chain and the has_retval crap > in plperl_handl_func(). Instead we let plperl_sv_to_datum() do most of > the lifting. It also now handles VOIDOID and checks that the request > result oid can be converted from the perl structure. For example if > you passed in a hashref with a result oid that was not an rowtype it > will error out with "PL/Perl cannot convert hash to non rowtype %s". > Arrays behave similarly. > > One side effect is you can now return a composite literal where you > could not before. ( We already let you return array literals ) > > The comments might still be a bit sparse-- Im hoping the added errors > make things a bit more self explanatory. > > A large portion of the diff is added regression tests, testing what > happens when you return various references. > > Comments? Looks good at first sight and passes all regression tests for me. -- Alexey Klyukinhttp://www.commandprompt.com The PostgreSQL Company – Command Prompt, Inc. -- 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] Online base backup from the hot-standby
On Mon, Oct 10, 2011 at 3:56 AM, Simon Riggs wrote: > 2011/10/9 Jun Ishiduka : > >> Insert WAL including a value of current FPW (on master) >> * In the the same timing as update, they insert WAL (is named >> XLOG_FPW_CHANGE). XLOG_FPW_CHANGE has a value of the changed FPW. >> * When it creates CHECKPOINT, it adds a value of current FPW to the >> CHECKPOINT WAL. > > I can't see a reason why we would use a new WAL record for this, > rather than modify the XLOG_PARAMETER_CHANGE record type which was > created for a very similar reason. > The code would be much simpler if we just extend > XLOG_PARAMETER_CHANGE, so please can we do that? After reading Ishiduka-san's patch, I'm thinking the opposite because (1) Whenever full_page_writes must be WAL-logged, there is no need to WAL-log the HS parameters. The opposite is also true. (2) How full_page_writes record should be replayed is quite different from how HS parameters record is. So ISTM that the code would be simpler if we introduce new WAL record for full_page_writes. Thought? Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Additional supplied modules
I've been thinking about the terminology used for various tools and extensions in PostgreSQL. The documentation bundles all them together in "Additional Supplied Modules, but really some are tools, some are libraries and some are extensions: Tools: oid2name pg_archivecleanup pgbench pg_standby pg_test_fsync pg_upgrade vacuumlo Libraries: auth_delay auto_explain dummy_seclabel passwordcheck sepgsql Extensions: adminpack btree_gin btree_gist chkpass citext cube dblink dict_int dict_xsyn earthdistance file_fdw fuzzystrmatch hstore intagg intarray isn lo ltree pageinspect pg_buffercache pgcrypto pg_freespacemap pgrowlocks pg_stat_statements pgstattuple pg_trgm seg sslinfo tablefunc test_parser tsearch2 unaccent uuid-ossp xml2 Extension Packages: spi (contains extensions timetravel, autoinc, insert_username, moddatetime) In fact if someone has been told to install one of those extensions in the SPI package, they might not know that it contains separate extensions, so calling CREATE EXTENSION spi; will do nothing. On top of this, tools will just exist and can be used without any configuration, whereas libraries will need to be loaded either with LOAD or in shared_preload_libraries, and extensions require CREATE EXTENSION. It seems very messy for all these to be bundled into the same section as if they were all equals. And the completely random nature of the naming conventions doesn't help either. (e.g. pg_stat_statements vs pgstattuple). Could we somehow categorise these, and also do something to clarify that SPI is a collection of extensions rather than an extension itself? -- Thom Brown Twitter: @darkixion IRC (freenode): dark_ixion Registered Linux user: #516935 EnterpriseDB UK: 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] [REVIEW] pg_last_xact_insert_timestamp
On Tue, Oct 4, 2011 at 9:15 AM, Simon Riggs wrote: >> Simon, could you? If your proposal turns out to be better than mine, I'd be >> happy to agree to drop my patch and adopt yours. > > Yes, will do. Simon, I believe that we are still waiting for this. Thanks, -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ts_rank
I'm sorry, my plane to Nepal is waiting me :) I'll be back in the midst of November. In short, ts_rank is based only on frequencies of lexems and doesn't count distance between query lexems. Also, it supports only primitive queries. Oleg On Wed, 12 Oct 2011, Bruce Momjian wrote: Bruce Momjian wrote: Mark wrote: There's some potentially useful information here: http://www.postgresql.org/docs/9.0/interactive/textsearch-controls.html#TEXTSEARCH-RANKING Thanks for reply. I was reading the documentation of PostgreSQL, but there it is not written the name of the used methods. Everywhere there is written, that ts_rank use standard ranking function. But it is difficult to say which is the standard function. Somewhere I found that it is maybe based on Vector space model and it seems to be truth, because in the code of tsrank.c is counted the frequency of words, but I am not sure of that :-( Oleg, Teodor, can you give me a description of how ts_rank decided how to rank items? Thanks. Any news on this question? Regards, Oleg _ Oleg Bartunov, Research Scientist, Head of AstroNet (www.astronet.ru), Sternberg Astronomical Institute, Moscow University, Russia Internet: o...@sai.msu.su, http://www.sai.msu.su/~megera/ phone: +007(495)939-16-83, +007(495)939-23-83 -- 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] [v9.2] Object access hooks with arguments support (v1)
Robert, I agree with it is a reasonable argument that compiler cannot raise warnings if all the arguments are delivered as Datum. In fact, I also tried to implement this feature with InvokeObjectAccessHook() defined as function. The first needed point to be improved is that we hope compiler to raise warnnings if and when number or type of arguments are changed in the future version. If so, how about an idea to define data types to inform modules the context information in, such as: struct ObjectAccessInfoData { ObjectAccessType oa_type; ObjectAddress oa_address; union { struct { HeapTuple newtuple; TupleDesc tupdesc; /* only if create a new relation */ : } post_create; /* additional info for OAT_POST_CREATE event */ } } Even if its invocation shall be wrapped up by macro, compiler can raise warnings when caller set values on the ObjectAccessInfoData structure. It will also help the second points partially. The objectaccess.h will perform a placeholder to describe specification of the argument. That clearly informs developers what informations are available on the hook and what was lacked. > Moreover, if > you did document it, I think it would boil down to "this is what > sepgsql happens to need", and I don't think that's an acceptable > answer. ?We have repeatedly refused to adopt that approach in the > past. > Unfortunately, I still don't have a clear answer for this point. However, in general terms, it is impossible to design any interface without knowledge of its usage more or less. We have several other hooks being available to loadable modules, but I don't believe that we designed these hooks without knowledge of use-cases, more or less. At least, this proposition enables modules being informed using commonly used data type (such as HeapTuple, TupleDesc), compared to the past approach that tried to push all the necessary information into argument list individually. I think the answer of this matter is on the middle-of-position between "we should not know anything about sepgsql" and "we should know everything about sepgsql", neither of them > (This particular case is worse than average, because new_rel_tup is > coming from InsertPgClassTuple, which therefore has to be modified, > along with AddNewRelationTuple and index_create, to get the tuple back > up to the call site where, apparently, it is needed.) > It might be a wrong design. All we want to inform was stored within new_rel_desc in heap_create_with_catalog(). So, I should design the hook to deliver new_rel_desc, instead of HeapTuple itself that need to pull up from InsertPgClassTuple. Thanks, 2011/10/12 Robert Haas : > On Thu, Sep 29, 2011 at 4:52 PM, Kohei KaiGai wrote: >> I noticed that the previous revision does not provide any way to inform >> the modules name of foreign server, even if foreign table was created, >> on the OAT_POST_CREATE hook. >> So, I modified the invocation at heap_create_with_catalog to deliver >> this information to the modules. >> >> Rest of parts were unchanged, except for rebasing to the latest git >> master. > > I've never really been totally sanguine with the idea of making object > access hooks take arguments, and all of my general concerns seem to > apply to the way you've set this patch up. In particular: > > 1. Type safety goes out the window. What you're essentially proposing > here is that we should have one hook function can be used for almost > anything at all and can be getting up to 9 arguments of any type > whatsoever. The hook will need to know how to interpret all of its > arguments depending on the context in which it was called. The > compiler will be totally unable to do any static type-checking, so > there will be absolutely no warning if the interface is changed > incompatibly on either side. Instead, you'll probably just crash the > database server at runtime. > > 2. The particular choice of data being passed to the object access > hooks appears capricious and arbitrary. Here's an example: > > /* Post creation hook for new relation */ > - InvokeObjectAccessHook(OAT_POST_CREATE, RelationRelationId, relid, 0); > + InvokeObjectAccessHook4(OAT_POST_CREATE, > + > RelationRelationId, relid, 0, > + > PointerGetDatum(new_rel_tup), > + > PointerGetDatum(tupdesc), > + > BoolGetDatum(is_select_into), > + > CStringGetDatum(fdw_srvname)); > > Now, I am sure that you have good reasons for wanting to pass those > particular things to the object access hook rather than any other > local variable or argument that might happen to be lying around at > this point in the code, but they are not documented. If someone adds > a new argument to this function, or removes an argument that's being > passed, they will have no idea what to do about this. Moreover, if > you did document it, I think it would boil down to "this is what > sepgsq
Re: [HACKERS] Online base backup from the hot-standby
> > > > > ERROR: full_page_writes on master is set invalid at least once since > > > > latest checkpoint > > > > > > > > I think this error should be rewritten as > > > > ERROR: full_page_writes on master has been off at some point since > > > > latest checkpoint > > > > > > > > We should be using 'off' instead of 'invalid' since that is what is what > > > > the user sets it to. > > > > > > Sure. > > > > What about the following message? It sounds more precise to me. > > > > ERROR: WAL generated with full_page_writes=off was replayed since last > > restartpoint > > Okay, I changes the patch to this messages. > If someone says there is a idea better than it, I will consider again. > > > > > I updated to patch corresponded above-comments. > > > > Thanks for updating the patch! Here are the comments: > > > > * don't yet have the insert lock, forcePageWrites could change under > > us, > > * but we'll recheck it once we have the lock. > > */ > > - doPageWrites = fullPageWrites || Insert->forcePageWrites; > > + doPageWrites = Insert->fullPageWrites || Insert->forcePageWrites; > > > > The source comment needs to be modified. > > > > * just turned off, we could recompute the record without full pages, > > but > > * we choose not to bother.) > > */ > > - if (Insert->forcePageWrites && !doPageWrites) > > + if ((Insert->fullPageWrites || Insert->forcePageWrites) && > > !doPageWrites) > > > > Same as above. > > Sure. > > > > XLogReportParameters() should skip writing WAL if full_page_writes has not > > been > > changed by SIGHUP. > > > > XLogReportParameters() should skip updating pg_control if any parameter > > related > > to hot standby has not been changed. > > YES. > > > > In checkpoint, XLogReportParameters() is called only when wal_level is > > hot_standby. > > OTOH, in walwriter, it's always called even when wal_level is not > > hot_standby. > > Can't we skip calling XLogReportParameters() whenever wal_level is not > > hot_standby? > > Yes, It is possible. > > > > In do_pg_start_backup() and do_pg_stop_backup(), the spinlock must be held > > to > > see XLogCtl->lastFpwDisabledLSN. > > Yes. > > > > What about changing the error message to: > > ERROR: WAL generated with full_page_writes=off was replayed during online > > backup > > Okay, too. > Sorry. > I was not previously able to answer fujii's all comments. > This is the remaining answers. > > > > + LWLockAcquire(WALInsertLock, LW_EXCLUSIVE); > > + XLogCtl->Insert.fullPageWrites = fullPageWrites; > > + LWLockRelease(WALInsertLock); > > > > I don't think WALInsertLock needs to be hold here because there is no > > concurrently running process which can access Insert.fullPageWrites. > > For example, Insert->currpos and Insert->LogwrtResult are also changed > > without the lock there. > > > > Yes. > > > The source comment of XLogReportParameters() needs to be modified. > > Yes, too. Done. I updated to patch corresponded above-comments. Regards. Jun Ishizuka NTT Software Corporation TEL:045-317-7018 E-Mail: ishizuka@po.ntts.co.jp standby_online_backup_09base-04fpw.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] patch: move dumpUserConfig call in dumpRoles function of pg_dumpall.c
On Wed, Oct 12, 2011 at 02:27:51PM -0400, Phil Sorber wrote: > My proposal would be to table the discussion about the 'ALTER DATABASE > SET ROLE' case because there seems to be a bit of a philosophical > debate behind this that needs to be sorted out first. Note: "to table the discussion" means opposite things in American and British english. I think the rest of the sentence makes it clear what you mean though :). Have a nice day, -- Martijn van Oosterhout http://svana.org/kleptog/ > He who writes carelessly confesses thereby at the very outset that he does > not attach much importance to his own thoughts. -- Arthur Schopenhauer signature.asc Description: Digital signature