Re: [HACKERS] Freeze avoidance of very large table.
On Sun, Apr 5, 2015 at 8:21 AM, Jeff Janes wrote: > On Sat, Apr 4, 2015 at 3:10 PM, Jim Nasby wrote: >> >> On 4/3/15 12:59 AM, Sawada Masahiko wrote: >>> >>> + case HEAPTUPLE_LIVE: >>> + case HEAPTUPLE_RECENTLY_DEAD: >>> + case HEAPTUPLE_INSERT_IN_PROGRESS: >>> + case HEAPTUPLE_DELETE_IN_PROGRESS: >>> + if >>> (heap_prepare_freeze_tuple(tuple.t_data, freezelimit, >>> + >>> mxactcutoff, &frozen[nfrozen])) >>> + frozen[nfrozen++].offset >>> = offnum; >>> + break; >> >> >> This doesn't seem safe enough to me. Can't there be tuples that are still >> new enough that they can't be frozen, and are still live? > > > Yep. I've set a table to read only while it contained unfreezable tuples, > and the tuples remain unfrozen yet the read-only action claims to have > succeeded. > > >> >> Somewhat related... instead of forcing the freeze to happen synchronously, >> can't we set this up so a table is in one of three states? Read/Write, Read >> Only, Frozen. AT_SetReadOnly and AT_SetReadWrite would simply change to the >> appropriate state, and all the vacuum infrastructure would continue to >> process those tables as it does today. lazy_vacuum_rel would become >> responsible for tracking if there were any non-frozen tuples if it was also >> attempting a freeze. If it discovered there were none, AND the table was >> marked as ReadOnly, then it would change the table state to Frozen and set >> relfrozenxid = InvalidTransactionId and relminxid = InvalidMultiXactId. >> AT_SetReadWrite could change relfrozenxid to it's own Xid as an >> optimization. Doing it that way leaves all the complicated vacuum code in >> one place, and would eliminate concerns about race conditions with still >> running transactions, etc. > > > +1 here as well. I might want to set tables to read only for reasons other > than to avoid repeated freezing. (After all, the name of the command > suggests it is a general purpose thing) and wouldn't want to automatically > trigger a vacuum freeze in the process. > Thank you for comments. > Somewhat related... instead of forcing the freeze to happen synchronously, > can't we set this up so a table is in one of three states? Read/Write, Read > Only, Frozen. AT_SetReadOnly and AT_SetReadWrite would simply change to > the > appropriate state, and all the vacuum infrastructure would continue to > process those tables as it does today. lazy_vacuum_rel would become > responsible for tracking if there were any non-frozen tuples if it was also > attempting > a freeze. If it discovered there were none, AND the table was > marked as ReadOnly, then it would change the table state to Frozen and set > relfrozenxid = InvalidTransactionId and relminxid = InvalidMultiXactId. > AT_SetReadWrite > could change relfrozenxid to it's own Xid as an > optimization. Doing it that way leaves all the complicated vacuum code in one > place, and would eliminate concerns about race conditions with still running > transactions, etc. I agree with 3 status, Read/Write, ReadOnly and Frozen. But I'm not sure when we should do to freeze tuples, e.g., scan whole tables. I think that the any changes to table are completely ignored/restricted if table is marked as ReadOnly table, and it's accompanied by freezing tuples, just mark as ReadOnly. Frozen table ensures that all tuples of its table completely has been frozen, so it also needs to scan whole table as well. e.g., we should need to scan whole table at two times. right? > +1 here as well. I might want to set tables to read only for reasons other > than to avoid repeated freezing. (After all, the name of the command > suggests it is a general purpose thing) and wouldn't want to automatically > trigger a > vacuum freeze in the process. > > There is another possibility here, too. We can completely divorce a ReadOnly > mode (which I think is useful for other things besides freezing) from the > question of whether we need to force-freeze a relation if we create a > FrozenMap, similar to the visibility map. This has the added advantage of > helping freeze scans on relations that are not ReadOnly in the case of tables > that are insert-mostly or any other pattern where most pages stay all-frozen. > Prior to the visibility map this would have been a rather daunting project, > but I believe this could piggyback on the VM code rather nicely. Anytime you > clear the VM you clearly must clear the FrozenMap as well. The logic for > setting the FM is clearly different, but that would be entirely > self-contained to vacuum. Unlike the VM, I don't see any point to marking > special bits in the page itself for FM. I was thinking this idea (FM) to avoid freezing all tuples actually. As you said, it might not be good idea (or overkill) that
Re: [HACKERS] pg_rewind and log messages
On Mon, Apr 6, 2015 at 12:57 PM, Fujii Masao wrote:
> (1)
> It outputs an error message to stdout not stderr.
> (2)
> The tool name should be added at the head of log message as follows,
> but not in pg_rewind.
>
> pg_basebackup: no target directory specified
Agreed. That's inconsistent.
> (3)
>>if (datadir_source == NULL && connstr_source == NULL)
>>{
>>pg_fatal("no source specified (--source-pgdata or
>> --source-server)\n");
>>pg_fatal("Try \"%s --help\" for more information.\n", progname);
>>exit(1);
>
> Since the first call of pg_fatal exits with 1, the subsequent pg_fatal and
> exit
> will never be called.
True.
> (4)
> ISTM that set_pglocale_pgservice() needs to be called, but not in pg_rewind.
True. I think that we should consider a call to get_restricted_token()
as well now that I look at it.
> (5)
> printf() is used to output an error in some files, e.g., timeline.c and
> parsexlog.c. These printf() should be replaced with pg_log or pg_fatal?
On top of that, datapagemap.c has a debug message as well using
printf, as well as filemap.c in print_filemap().
I guess that you are working on a patch? If not, you are looking for one?
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
[HACKERS] pg_rewind and log messages
Hi,
I found that pg_rewind has several problems about its log messages.
(1)
It outputs an error message to stdout not stderr.
(2)
The tool name should be added at the head of log message as follows,
but not in pg_rewind.
pg_basebackup: no target directory specified
(3)
>if (datadir_source == NULL && connstr_source == NULL)
>{
>pg_fatal("no source specified (--source-pgdata or --source-server)\n");
>pg_fatal("Try \"%s --help\" for more information.\n", progname);
>exit(1);
Since the first call of pg_fatal exits with 1, the subsequent pg_fatal and exit
will never be called.
(4)
ISTM that set_pglocale_pgservice() needs to be called, but not in pg_rewind.
(5)
printf() is used to output an error in some files, e.g., timeline.c and
parsexlog.c. These printf() should be replaced with pg_log or pg_fatal?
Regards,
--
Fujii Masao
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Doubt about AccessExclusiveLock in ALTER TABLE .. SET ( .. );
Fabrízio de Royes Mello wrote: > Ok guys. The attached patch refactor the reloptions adding a new field > "lockmode" in "relopt_gen" struct and a new method to determine the > required lock level from an option list. > > We need determine the appropriate lock level for each reloption: I don't think AccessShareLock is appropriate for any option change. You should be using a lock level that's self-conflicting, as a minimum requirement, to avoid two processes changing the value concurrently. (I would probably go as far as ensuring that the lock level specified in the table DoLockModesConflict() with itself in an Assert somewhere.) -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Upper-case error in docs regarding PQmakeEmptyPGresult
On Mon, Apr 6, 2015 at 11:40 AM, Michael Paquier wrote: > Hi all, > > I just spotted that PQmakeEmptyPGresult is named PQmakeEmptyPGResult > in one place (not "Result", but "result") in the docs. > A patch is attached. Pushed. Thanks! Regards, -- Fujii Masao -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Doubt about AccessExclusiveLock in ALTER TABLE .. SET ( .. );
On Wed, Apr 1, 2015 at 1:45 AM, Noah Misch wrote:
>
> On Tue, Mar 31, 2015 at 01:17:03PM -0400, Robert Haas wrote:
> > On Tue, Mar 31, 2015 at 9:11 AM, Fabrízio de Royes Mello
> > wrote:
> > > Attached a very WIP patch to reduce lock level when setting autovacuum
> > > reloptions in "ALTER TABLE .. SET ( .. )" statement.
> >
> > I think the first thing we need to here is analyze all of the options
> > and determine what the appropriate lock level is for each, and why.
>
> Agreed. Fabrízio, see this message for the discussion that led to the
code
> comment you found (search for "relopt_gen"):
>
>
http://www.postgresql.org/message-id/20140321034556.ga3927...@tornado.leadboat.com
Ok guys. The attached patch refactor the reloptions adding a new field
"lockmode" in "relopt_gen" struct and a new method to determine the
required lock level from an option list.
We need determine the appropriate lock level for each reloption:
- boolRelopts:
* autovacuum_enabled (AccessShareLock)
* user_catalog_table (AccessExclusiveLock)
* fastupdate (AccessExclusiveLock)
* security_barrier (AccessExclusiveLock)
- intRelOpts:
* fillfactor (heap) (AccessExclusiveLock)
* fillfactor (btree) (AccessExclusiveLock)
* fillfactor (gist) (AccessExclusiveLock)
* fillfactor (spgist) (AccessExclusiveLock)
* autovacuum_vacuum_threshold (AccessShareLock)
* autovacuum_analyze_threshold (AccessShareLock)
* autovacuum_vacuum_cost_delay (AccessShareLock)
* autovacuum_vacuum_cost_limit (AccessShareLock)
* autovacuum_freeze_min_age (AccessShareLock)
* autovacuum_multixact_freeze_min_age (AccessShareLock)
* autovacuum_freeze_max_age (AccessShareLock)
* autovacuum_multixact_freeze_max_age (AccessShareLock)
* autovacuum_freeze_table_age (AccessShareLock)
* autovacuum_multixact_freeze_table_age (AccessShareLock)
* log_autovacuum_min_duration (AccessShareLock)
* pages_per_range (AccessExclusiveLock)
* gin_pending_list_limit (AccessExclusiveLock)
- realRelOpts:
* autovacuum_vacuum_scale_factor (AccessShareLock)
* autovacuum_analyze_scale_factor (AccessShareLock)
* seq_page_cost (AccessExclusiveLock)
* random_page_cost (AccessExclusiveLock)
* n_distinct (AccessExclusiveLock)
* n_distinct_inherited (AccessExclusiveLock)
- stringRelOpts:
* buffering (AccessExclusiveLock)
* check_option (AccessExclusiveLock)
In the above list I just change lock level from AccessExclusiveLock to
AccessShareLock to all "autovacuum" related reloptions because it was the
motivation of this patch.
I need some help to define the others.
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/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index 8176b6a..f83ce2f 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -57,7 +57,8 @@ static relopt_bool boolRelOpts[] =
{
"autovacuum_enabled",
"Enables autovacuum in this relation",
- RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
+ RELOPT_KIND_HEAP | RELOPT_KIND_TOAST,
+ AccessExclusiveLock
},
true
},
@@ -65,7 +66,8 @@ static relopt_bool boolRelOpts[] =
{
"user_catalog_table",
"Declare a table as an additional catalog table, e.g. for the purpose of logical replication",
- RELOPT_KIND_HEAP
+ RELOPT_KIND_HEAP,
+ AccessExclusiveLock
},
false
},
@@ -73,7 +75,8 @@ static relopt_bool boolRelOpts[] =
{
"fastupdate",
"Enables \"fast update\" feature for this GIN index",
- RELOPT_KIND_GIN
+ RELOPT_KIND_GIN,
+ AccessExclusiveLock
},
true
},
@@ -81,7 +84,8 @@ static relopt_bool boolRelOpts[] =
{
"security_barrier",
"View acts as a row security barrier",
- RELOPT_KIND_VIEW
+ RELOPT_KIND_VIEW,
+ AccessExclusiveLock
},
false
},
@@ -95,7 +99,8 @@ static relopt_int intRelOpts[] =
{
"fillfactor",
"Packs table pages only to this percentage",
- RELOPT_KIND_HEAP
+ RELOPT_KIND_HEAP,
+ AccessExclusiveLock
},
HEAP_DEFAULT_FILLFACTOR, HEAP_MIN_FILLFACTOR, 100
},
@@ -103,7 +108,8 @@ static relopt_int intRelOpts[] =
{
"fillfactor",
"Packs btree index pages only to this percentage",
- RELOPT_KIND_BTREE
+ RELOPT_KIND_BTREE,
+ AccessExclusiveLock
},
BTREE_DEFAULT_FILLFACTOR, BTREE_MIN_FILLFACTOR, 100
},
@@ -111,7 +117,8 @@ static relopt_int intRelOpts[] =
{
"fillfactor",
"Packs hash index pages only to this percentage",
- RELOPT_KIND_HASH
+ RELOPT_KIND_HASH,
+ AccessExclusiveLock
},
HASH_DEFAULT_FILLFACTOR, HASH_MIN_FILLFACTOR, 100
},
@@ -119,7 +126,8 @@ static relopt_int intRelOpts[] =
{
"fillfactor",
"Packs gist index pages only to this percentage",
- RELOPT_KIND_GIST
+ RELOP
[HACKERS] Re: [HACKERS] GSoC 2015 proposal: Improve the performance of “ALTER TABLE .. SET LOGGED / UNLOGGED” statement
On Thu, Apr 2, 2015 at 3:24 PM, Robert Haas wrote: > > On Wed, Mar 25, 2015 at 9:46 PM, Fabrízio de Royes Mello > wrote: > > http://www.postgresql.org/message-id/ca+tgmozm+-0r7h0edpzzjbokvvq+gavkchmno4fypveccw-...@mail.gmail.com > > I like the idea of the feature a lot, but the proposal to which you > refer here mentions some problems, which I'm curious how you think you > might solve. (I don't have any good ideas myself, beyond what I > mentioned there.) > How the cleanup will happens? Regards, -- Fabrízio de Royes Mello Consultoria/Coaching PostgreSQL >> Timbira: http://www.timbira.com.br >> Blog: http://fabriziomello.github.io >> Linkedin: http://br.linkedin.com/in/fabriziomello >> Twitter: http://twitter.com/fabriziomello >> Github: http://github.com/fabriziomello
[HACKERS] Upper-case error in docs regarding PQmakeEmptyPGresult
Hi all, I just spotted that PQmakeEmptyPGresult is named PQmakeEmptyPGResult in one place (not "Result", but "result") in the docs. A patch is attached. Regards, -- Michael diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml index c08e6b6..b5533cd 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -5793,7 +5793,7 @@ int PQfireResultCreateEvents(PGconn *conn, PGresult *res); The main reason that this function is separate from - PQmakeEmptyPGResult is that it is often appropriate + PQmakeEmptyPGresult is that it is often appropriate to create a PGresult and fill it with data before invoking the event procedures. -- 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: Reducing lock strength of trigger and foreign key DDL
On 5 April 2015 at 19:19, Michael Paquier wrote: > Cool! Thanks for showing up. Visibility <> Activity. How is REINDEX CONCURRENTLY doing? -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, RemoteDBA, 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] PATCH: Reducing lock strength of trigger and foreign key DDL
On 04/05/2015 05:56 PM, Simon Riggs wrote: Committed. We move forwards, slowly but surely. Thanks for the patch. Thanks to you and the reviewers for helping me out with this patch! -- Andreas Karlsson -- 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: Reducing lock strength of trigger and foreign key DDL
On Mon, Apr 6, 2015 at 12:56 AM, Simon Riggs wrote: > On 7 February 2015 at 20:05, Andreas Karlsson wrote: >> On 01/30/2015 07:48 AM, Michael Paquier wrote: >>> >>> Looking at the latest patch, it seems that in >>> AlterTableGetLockLevel@tablecmds.c we ought to put AT_ReAddConstraint, >>> AT_AddConstraintRecurse and AT_ProcessedConstraint under the same >>> banner as AT_AddConstraint. Thoughts? >> >> >> A new version of the patch is attached which treats them as the same for >> locking. I think it is correct and improves readability to do so. > > Committed. We move forwards, slowly but surely. Thanks for the patch. Cool! Thanks for showing up. -- 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] PATCH: Spinlock Documentation
Hello, Thank you very much for your feedback! I will work on the changes as soon as I can. Respectfully, Artem Luzyanin On Sunday, April 5, 2015 5:45 PM, Tom Lane wrote: David Fetter writes: > One issue with this patch is that it is not localized. If someone > goes and changes the S_LOCK implementation for one of the platforms > below, or adds a new platform, etc., without changing this comment > too, this comment becomes confusingly obsolete. Indeed. Moreover, this header comment is supposed to be an overview and specification of the macros that need to be provided. I think it's an actively bad idea to clutter it with platform-by-platform details; that will create a "can't see the forest for the trees" problem. If we need more info here, I think a comment block before each section of the file would make more sense. But the patch as provided seems like it would just be redundant if it were refactored in that form. What would possibly be useful that's not there now is a paragraph or two describing the overall layout of the file (eg "gcc then non gcc", or whatever can be said at more or less that level of detail). But please don't stick that into the middle of the specification part. regards, tom lane
Re: [HACKERS] PATCH: Spinlock Documentation
David Fetter writes: > One issue with this patch is that it is not localized. If someone > goes and changes the S_LOCK implementation for one of the platforms > below, or adds a new platform, etc., without changing this comment > too, this comment becomes confusingly obsolete. Indeed. Moreover, this header comment is supposed to be an overview and specification of the macros that need to be provided. I think it's an actively bad idea to clutter it with platform-by-platform details; that will create a "can't see the forest for the trees" problem. If we need more info here, I think a comment block before each section of the file would make more sense. But the patch as provided seems like it would just be redundant if it were refactored in that form. What would possibly be useful that's not there now is a paragraph or two describing the overall layout of the file (eg "gcc then non gcc", or whatever can be said at more or less that level of detail). But please don't stick that into the middle of the specification part. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PATCH: Spinlock Documentation
On Sun, Apr 05, 2015 at 06:50:59PM +, Artem Luzyanin wrote: > Hello, > > I am new to PostgreSQLcommunity, but I would like to become a > contributer eventually. I have readthrough your "Submitting Patch" > guide and decided to follow "Start with submitting a patch that is > small anduncontroversial to help them understand you, and to get you > familiar with theoverall process" suggestion. > > I am interested inplatform-specific spinlock implementation, so I > looked at s_lock.h file for possibleimprovement. Since it took me > some time to find possible areas of improvement,I would like to > submit a small patch that would facilitate the process forfuture > contributors (including myself). Since this is my first e-mail, > pleaselet me know if I should have done something differently in > order to submit apatch for the community. One issue with this patch is that it is not localized. If someone goes and changes the S_LOCK implementation for one of the platforms below, or adds a new platform, etc., without changing this comment too, this comment becomes confusingly obsolete. How do you plan to address this issue? Cheers, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com 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
[HACKERS] Bringing text abbreviation debug output under the control of trace_sort
Attached patch makes trace_sort control abbreviation debug output for
the text opclass, which makes it consistent with the numeric opclass.
This seems better than relying on someone going to the trouble of
building Postgres themselves to debug cases where abbreviation is the
wrong thing, which we're not 100% sure will not occur. It also allows
wider analysis of where abbreviation helps the most and the least in
production, which is surely a good thing.
I have added this to the next commitfest, but I suggest that it be
quickly committed to the master branch as a maintenance commit.
--
Peter Geoghegan
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index d874a1a..18cbdd0 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -34,9 +34,6 @@
#include "utils/pg_locale.h"
#include "utils/sortsupport.h"
-#ifdef DEBUG_ABBREV_KEYS
-#define DEBUG_elog_output DEBUG1
-#endif
/* GUC variable */
int bytea_output = BYTEA_OUTPUT_HEX;
@@ -2149,11 +2146,13 @@ bttext_abbrev_abort(int memtupcount, SortSupport ssup)
* time there are differences within full key strings not captured in
* abbreviations.
*/
-#ifdef DEBUG_ABBREV_KEYS
+#ifdef TRACE_SORT
+ if (trace_sort)
{
double norm_abbrev_card = abbrev_distinct / (double) memtupcount;
- elog(DEBUG_elog_output, "abbrev_distinct after %d: %f (key_distinct: %f, norm_abbrev_card: %f, prop_card: %f)",
+ elog(LOG, "bttext_abbrev: abbrev_distinct after %d: %f "
+ "(key_distinct: %f, norm_abbrev_card: %f, prop_card: %f)",
memtupcount, abbrev_distinct, key_distinct, norm_abbrev_card,
tss->prop_card);
}
@@ -2219,11 +2218,11 @@ bttext_abbrev_abort(int memtupcount, SortSupport ssup)
* of moderately high to high abbreviated cardinality. There is little to
* lose but much to gain, which our strategy reflects.
*/
-#ifdef DEBUG_ABBREV_KEYS
- elog(DEBUG_elog_output, "would have aborted abbreviation due to worst-case at %d. abbrev_distinct: %f, key_distinct: %f, prop_card: %f",
- memtupcount, abbrev_distinct, key_distinct, tss->prop_card);
- /* Actually abort only when debugging is disabled */
- return false;
+#ifdef TRACE_SORT
+ if (trace_sort)
+ elog(LOG, "bttext_abbrev: aborted abbreviation at %d "
+ "(abbrev_distinct: %f, key_distinct: %f, prop_card: %f)",
+ memtupcount, abbrev_distinct, key_distinct, tss->prop_card);
#endif
return true;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] PATCH: Spinlock Documentation
Hello, I am new to PostgreSQLcommunity, but I would like to become a contributer eventually. I have readthrough your "Submitting Patch" guide and decided to follow "Start with submitting a patch that is small anduncontroversial to help them understand you, and to get you familiar with theoverall process" suggestion. I am interested inplatform-specific spinlock implementation, so I looked at s_lock.h file for possibleimprovement. Since it took me some time to find possible areas of improvement,I would like to submit a small patch that would facilitate the process forfuture contributors (including myself). Since this is my first e-mail, pleaselet me know if I should have done something differently in order to submit apatch for the community. Project name: Spinlock Documentation Uniquely identifiable file name: s_lock.h What the patch does: The patch implements addition to documentation in thementioned above file. This addition outlines the current platform-specificimplementations for an easy road map to what else could be done. Whether the patch is for discussion or forapplication: This patch is for application. Which branch the patch is against: This patch is against master branch. Whether it compiles and tests successfully: The changes allow for successful compilation andtesting. Whether it contains any platform-specificitems and if so, has it been tested on other platforms: This patch doesn’t have any platform-specific items. Confirm that the patch includes regression tests to check the newfeature actually works as described. Since this is documentation improvement, regressiontests are not needed. Include documentation on how to use the newfeature, including examples: Since it’s documentation improvement, nodocumentation is needed for documentation. Describe the effect your patch has onperformance, if any: No effect on performance. Unless we are talking aboutdeveloper’s performance. Try to include a few lines about why youchose to do things particular ways: I have decided to include the mentioned documentationto outline the areas that need improvement. Any developer, looking forplatform-specific code improvement implementation can now easily find theneeded area. Thankyou for your time and help, ArtemLuzyanin spinlock-docs.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
[HACKERS] Column mis-spelling hints vs case folding
A newbie error that we see *constantly* is misunderstanding identifier case-folding rules. ISTM that commit e529cd4ff missed a chance to help with that. You do get a hint for this: regression=# create table t1 (foo int, "Bar" int); CREATE TABLE regression=# select bar from t1; ERROR: column "bar" does not exist LINE 1: select bar from t1; ^ HINT: Perhaps you meant to reference the column "t1"."Bar". but apparently it's just treating that as a vanilla one-mistyped-character error, because there's no hint for this: regression=# create table t2 (foo int, "BAR" int); CREATE TABLE regression=# select BAR from t2; ERROR: column "bar" does not exist LINE 1: select BAR from t2; ^ I think this hint might be a lot more useful if its comparison mechanism were case-insensitive. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PATCH: Reducing lock strength of trigger and foreign key DDL
On 7 February 2015 at 20:05, Andreas Karlsson wrote: > On 01/30/2015 07:48 AM, Michael Paquier wrote: >> >> Looking at the latest patch, it seems that in >> AlterTableGetLockLevel@tablecmds.c we ought to put AT_ReAddConstraint, >> AT_AddConstraintRecurse and AT_ProcessedConstraint under the same >> banner as AT_AddConstraint. Thoughts? > > > A new version of the patch is attached which treats them as the same for > locking. I think it is correct and improves readability to do so. Committed. We move forwards, slowly but surely. Thanks for the patch. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, RemoteDBA, 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
