Re: [HACKERS] BUG #6510: A simple prompt is displayed using wrong charset
On 01/24/2013 01:34 AM, Tom Lane wrote: > Alexander Law writes: >> Please let me know if I can do something to get the bug fix >> (https://commitfest.postgresql.org/action/patch_view?id=902) committed. > It's waiting on some Windows-savvy committer to pick it up, IMO. I'm no committer, but I can work with Windows and know text encoding issues fairly well. I'll take a look, though I can't do it immediately as I have some other priority work. Should be able to in the next 24h. -- Craig Ringer 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] My first patch! (to \df output)
On 01/24/2013 01:50 AM, Phil Sorber wrote: > This looks good to me now. Compiles and works as described. Ready to go? https://commitfest.postgresql.org/action/patch_view?id=1008 -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Re: [HACKERS] bgwriter reference to HOT standby
On Thu, Jan 24, 2013 at 12:36 PM, Jeff Janes wrote: > The docs on bgworker twice refer to "HOT standby". I don't think that in > either case, the "hot" needs emphasis, and if it does making it look like an > acronym (one already used for something else) is probably not the way to do > it. I think it should it be "Hot Standby" for consistency. +1 for changing it from HOT to hot/Hot anyway Thanks, Pavan -- Pavan Deolasee http://www.linkedin.com/in/pavandeolasee -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] bgwriter reference to HOT standby
The docs on bgworker twice refer to "HOT standby". I don't think that in either case, the "hot" needs emphasis, and if it does making it look like an acronym (one already used for something else) is probably not the way to do it. Patch to HEAD attached. Cheers, Jeff diff --git a/doc/src/sgml/bgworker.sgml b/doc/src/sgml/bgworker.sgml new file mode 100644 index 912c7de..1151161 *** a/doc/src/sgml/bgworker.sgml --- b/doc/src/sgml/bgworker.sgml *** typedef struct BackgroundWorker *** 74,84 postgres itself has finished its own initialization; processes requesting this are not eligible for database connections), BgWorkerStart_ConsistentState (start as soon as a consistent state !has been reached in a HOT standby, allowing processes to connect to databases and run read-only queries), and BgWorkerStart_RecoveryFinished (start as soon as the system has entered normal read-write state). Note the last two values are equivalent !in a server that's not a HOT standby. Note that this setting only indicates when the processes are to be started; they do not stop when a different state is reached. --- 74,84 postgres itself has finished its own initialization; processes requesting this are not eligible for database connections), BgWorkerStart_ConsistentState (start as soon as a consistent state !has been reached in a hot standby, allowing processes to connect to databases and run read-only queries), and BgWorkerStart_RecoveryFinished (start as soon as the system has entered normal read-write state). Note the last two values are equivalent !in a server that's not a hot standby. Note that this setting only indicates when the processes are to be started; they do not stop when a different state is reached. -- 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] Prepared statements fail after schema changes with surprising error
Robert Haas writes:
> On Wed, Jan 23, 2013 at 11:40 AM, Tom Lane wrote:
>> Your point that the locking code doesn't quite cope with newly-masked
>> objects makes me feel that we could get away with not solving the case
>> for plan caching either. Or at least that we could put off the problem
>> till another day. If we are willing to just change plancache's handling
>> of search_path, that's a small patch that I think is easily doable for
>> 9.3. If we insist on installing schema-level invalidation logic, it's
>> not happening before 9.4.
> I agree with that analysis. FWIW, I am pretty confident that the
> narrower fix will make quite a few people significantly happier than
> they are today, so if you're willing to take that on, +1 from me. I
> believe the search-path-interpolation problem is a sufficiently
> uncommon case that, in practice, it rarely comes up. That's not to
> say that we shouldn't ever fix it, but I think the simpler fix will be
> a 90% solution and people will be happy to have made that much
> progress this cycle.
Here's a draft patch for that. I've not looked yet to see if there's
any documentation that ought to be touched.
With this patch, PushOverrideSearchPath/PopOverrideSearchPath are used
in only one place: CreateSchemaCommand. And that's a very simple,
stylized usage: temporarily push the new schema onto the front of the
path. It's tempting to think about replacing that klugy code with some
simpler mechanism. But that sort of cleanup should probably be a
separate patch.
regards, tom lane
diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index b256498d4511add3f55c35f07fecf6a92f19e763..ca4635dc51f41b050521d65cd601bccab90b6726 100644
*** a/src/backend/catalog/namespace.c
--- b/src/backend/catalog/namespace.c
*** CopyOverrideSearchPath(OverrideSearchPat
*** 3097,3102
--- 3097,3124
}
/*
+ * OverrideSearchPathMatchesCurrent - does path match current setting?
+ */
+ bool
+ OverrideSearchPathMatchesCurrent(OverrideSearchPath *path)
+ {
+ /* Easiest way to do this is GetOverrideSearchPath() and compare */
+ bool result;
+ OverrideSearchPath *cur;
+
+ cur = GetOverrideSearchPath(CurrentMemoryContext);
+ if (path->addCatalog == cur->addCatalog &&
+ path->addTemp == cur->addTemp &&
+ equal(path->schemas, cur->schemas))
+ result = true;
+ else
+ result = false;
+ list_free(cur->schemas);
+ pfree(cur);
+ return result;
+ }
+
+ /*
* PushOverrideSearchPath - temporarily override the search path
*
* We allow nested overrides, hence the push/pop terminology. The GUC
diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c
index cbc7c498d0d533b149fe4395992d9910a2d6b5cd..4630c44e7407c98a2de2df7bafc2753e78bfcfc6 100644
*** a/src/backend/utils/cache/plancache.c
--- b/src/backend/utils/cache/plancache.c
***
*** 15,27
* that matches the event is marked invalid, as is its generic CachedPlan
* if it has one. When (and if) the next demand for a cached plan occurs,
* parse analysis and rewrite is repeated to build a new valid query tree,
! * and then planning is performed as normal.
*
* Note that if the sinval was a result of user DDL actions, parse analysis
* could throw an error, for example if a column referenced by the query is
! * no longer present. The creator of a cached plan can specify whether it
! * is allowable for the query to change output tupdesc on replan (this
! * could happen with "SELECT *" for example) --- if so, it's up to the
* caller to notice changes and cope with them.
*
* Currently, we track exactly the dependencies of plans on relations and
--- 15,29
* that matches the event is marked invalid, as is its generic CachedPlan
* if it has one. When (and if) the next demand for a cached plan occurs,
* parse analysis and rewrite is repeated to build a new valid query tree,
! * and then planning is performed as normal. We also force re-analysis and
! * re-planning if the active search_path is different from the previous time.
*
* Note that if the sinval was a result of user DDL actions, parse analysis
* could throw an error, for example if a column referenced by the query is
! * no longer present. Another possibility is for the query's output tupdesc
! * to change (for instance "SELECT *" might expand differently than before).
! * The creator of a cached plan can specify whether it is allowable for the
! * query to change output tupdesc on replan --- if so, it's up to the
* caller to notice changes and cope with them.
*
* Currently, we track exactly the dependencies of plans on relations and
*** CreateCachedPlan(Node *raw_parse_tree,
*** 174,184
plansource->cursor_options = 0;
plansource->fixed_result = false;
plansource->resultDesc = NULL;
- plansource->search_path = NULL;
plansource->context = source_context;
Re: [HACKERS] pg_basebackup with -R option and start standby have problems with escaped password
On Wed, Jan 23, 2013 11:48 PM, Magnus Hagander wrote:
>On Wed, Jan 23, 2013 at 10:18 AM, Hari Babu
wrote:
>> Test scenario to reproduce:
>> 1. Start the server
>> 2. create the user as follows
>> ./psql postgres -c "create user user1 superuser login
>> password 'use''1'"
>>
>> 3. Take the backup with -R option as follows.
>> ./pg_basebackup -D ../../data1 -R -U user1 -W
>>
>> The following errors are occurring when the new standby on the backup
>> database starts.
>>
>> FATAL: could not connect to the primary server: missing "=" after "1'"
in
>> connection info string
>
>What does the resulting recovery.conf file look like?
The recovery.conf which is generated is as follows
standby_mode = 'on'
primary_conninfo = 'user=''user1'' password=''use''1'' port=''5432'' '
I observed the problem is while reading primary_conninfo from the
recovery.conf file
the function "GUC_scanstr" removes the quotes of the string and also makes
the
continuos double quote('') as single quote(').
By using the same connection string while connecting to primary server the
function "conninfo_parse" the escape quotes are not able to parse properly
and it is leading
to problem.
please correct me if any thing wrong in my observation.
Regards,
Hari babu.
--
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] Re: Proposal for Allow postgresql.conf values to be changed via SQL [review]
On Wednesday, January 23, 2013 11:51 PM Alvaro Herrera wrote: > Fujii Masao escribió: > > On Wed, Jan 23, 2013 at 12:24 PM, Amit Kapila > wrote: > > > >> Is it safe to write something in the directory other than data > > >> directory > > >> via SQL? > > >> > > >> postgres user usually has the write permission for the > configuration > > >> directory like /etc/postgresql? > > > > > > As postgresql.conf will also be in same path and if user can change > that, > > > then what's the problem with postgresql.auto.conf > > > > If the configuration directory like /etc is owned by root and only > root has > > a write permission for it, the user running PostgreSQL server would > not > > be able to update postgresql.auto.conf there. OTOH, even in that > case, > > a user can switch to root and update the configuration file there. > I'm not > > sure whether the configuration directory is usually writable by the > user > > running PostgreSQL server in Debian or Ubuntu, though. > > Yes it is -- the /etc/postgresql// directory (where > postgresql.conf resides) is owned by user postgres. So in that case we can consider postgresql.auto.conf to be in path w.r.t postgresql.conf instead of data directory. With Regards, Amit Kapila. -- 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] Re: Proposal for Allow postgresql.conf values to be changed via SQL [review]
On Wednesday, January 23, 2013 9:51 PM Fujii Masao wrote: > On Wed, Jan 23, 2013 at 6:18 PM, Amit Kapila > wrote: > > On Tuesday, January 22, 2013 10:14 PM Fujii Masao wrote: > >> When I removed postgresql.auto.conf and restarted the server, > >> I got the following warning message. This is not correct because > >> I didn't remove "auto.conf.d" from postgresql.conf. What I removed > >> is only postgresql.auto.conf. > >> > >> WARNING: Default "auto.conf.d" is not present in postgresql.conf. > >> Configuration parameters changed with SET PERSISTENT command will > not > >> be effective. > > > > How about changing it to below message: > > > > WARNING: File 'postgresql.auto.conf' is not accessible, either file > > 'postgresql.auto.conf' or folder '%s' doesn't exist or default > "auto.conf.d" > > is not present in postgresql.conf. > > Configuration parameters changed with SET PERSISTENT command will not > be > > effective. > > Or we should suppress such a warning message in the case where > postgresql.auto.conf doesn't exist? SET PERSISTENT creates that > file automatically if it doesn't exist. So we can expect that > configuration > parameters changed with SET PERSISTENT WILL be effective. This Warning (message) can come during startup or reload, so if at that time postgresql.auto.conf doesn't exist the parameters set by SET PERSISTENT previous to startup or reload will not be effective. So IMO, we can think to change part of this message as below: "Configuration parameters changed before start of server with SET PERSISTENT command will not be effective." Any other idea for change in message? > This warning message implies that the line "include_dir 'auto.conf.d'" > must not be removed from postgresql.conf? If so, we should warn that > in both document and postgresql.conf.sample? I shall fix this as below: Change current WARNING in postgresql.conf.sample as below: # This includes the default configuration directory included to support # SET PERSISTENT statement. # # WARNING: User should not remove below include_dir or directory auto.conf.d, as both are required to make SET PERSISTENT command work. Any configuration parameter values specified after this line # will override the values set by SET PERSISTENT statement. Also Update this in runtime.sgml under heading "Starting the Database Server" > Or we should hard-code > so that something like auto.conf.d is always included even when that > include_dir directive doesn't exist? I think this can create problem for user who intentionally wants to remove this directory inclusion. With Regards, Amit Kapila. -- 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] Teaching pg_receivexlog to follow timeline switches
On 01/22/2013 06:43 AM, Noah Misch wrote: > This patch was in Needs Review status, but you committed it on 2013-01-17. I > have marked it as such in the CF app. Thankyou. There's a lot to keep up with :S -- Craig Ringer 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] Visual Studio 2012 RC
On 01/24/2013 11:28 AM, Craig Ringer wrote: > On 01/24/2013 09:38 AM, Noah Misch wrote: >> The most notable difference is that I have no pre-VS2012 Microsoft >> compilers installed and no SDKs installed by my explicit action. I >> suggest assessing how the Framework64 directories get into your path >> and trying without them. nm > > Have you verified that 64-bit builds work? I'm testing now, but I've > just confirmed that my machine isn't quite right. OK, 64-bit builds with the x86_amd64 cross-compiler work. I cannot test native x64 builds as Microsoft no longer appear to release the native x64 compilers in any free SDK, but I see no reason there would be problems, nor any reason to hold back the work just in case. A 64-bit cross compile produces perfectly reasonable, working 64-bit binaries. I have some final revisions to propose for the documentation but otherwise this looks ready for commit. I don't like the section on the Windows SDK at all. With all the variations it's grown cumbersome and it's unnecessary to install for most people. It may even cause problems - building an old Pg against a new WinSDK may introduce compatibility issues. I propose to omit that section entirely, and instead amend the section that discusses VS versions and compilers to mention that you can build with: - VS 2008 to 2012 (no SDK required) - Microsoft SDK 6.0a to 7.1 with included compilers - VS 2005 + separately installed Microsoft SDK I'd really like to link to the wiki for details of how to set up each environment, as the details change when Microsoft releases new SDKs and breaks old ones, new workarounds turn up, etc. I know we don't usually link to the wiki from the docs, but I feel in this case it's justified. I can just mention "look for Windows installation information on the PostgreSQL wiki" but would prefer a direct link. Thankyou for documenting the locale issues. That will save somebody's sanity someday. I'd love to test the locale changes in detail, but available time doesn't permit that, and I don't see anything that will affect the behaviour of Pg when built with an older Visual Studio. I've attached a patch with my amendments to the documentation. I think that with that, it's ready to go, but I'd like your final approval of the docs changes before I flag it ready for commit. The patch doesn't link directly to the wiki; if everyone's OK with that I'd like to get this in now and add any changes like that as a separate revision. It's also been pushed to https://github.com/ringerc/postgres/tree/vs2012 -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services >From 70793f3228fcb487711703fbff1a7643fe10c28e Mon Sep 17 00:00:00 2001 From: Craig Ringer Date: Mon, 21 Jan 2013 20:45:02 +0800 Subject: [PATCH] Visual Studio 2012 (VS 11) build support by Brar Piening and Noah Misch This patch introduces handling of the Windows Vista / VS 2012 setlocale() changes and the altered contents of _locale_t; see comments in src/backend/utils/adt/pg_locale.c and http://www.postgresql.org/message-id/20130101025421.ga17...@tornado.leadboat.com Review and documentation amendments by Craig Ringer --- doc/src/sgml/install-windows.sgml | 96 +++ src/backend/utils/adt/pg_locale.c | 68 +-- src/bin/initdb/initdb.c | 12 - src/port/chklocale.c | 43 ++ src/port/win32env.c | 6 +++ src/tools/msvc/MSBuildProject.pm | 44 +- src/tools/msvc/README | 9 ++-- src/tools/msvc/Solution.pm| 31 +++-- src/tools/msvc/VSObjectFactory.pm | 14 -- src/tools/msvc/build.pl | 2 +- src/tools/msvc/gendef.pl | 1 + 11 files changed, 256 insertions(+), 70 deletions(-) diff --git a/doc/src/sgml/install-windows.sgml b/doc/src/sgml/install-windows.sgml new file mode 100644 index 452cf31..ea7e687 *** a/doc/src/sgml/install-windows.sgml --- b/doc/src/sgml/install-windows.sgml *** *** 19,26 There are several different ways of building PostgreSQL on Windows. The simplest way to build with ! Microsoft tools is to install a supported version of the ! Microsoft Windows SDK and use the included compiler. It is also possible to build with the full Microsoft Visual C++ 2005, 2008 or 2010. In some cases that requires the installation of the Windows SDK --- 19,26 There are several different ways of building PostgreSQL on Windows. The simplest way to build with ! Microsoft tools is to install Visual Studio Express 2012 ! for Windows Desktop and use the included compiler. It is also possible to build with the full Microsoft Visual C++ 2005, 2008 or 2010. In some cases that requires the installation of the Windows SDK *** *** 77,94 Visual Studio Express or some versions of the Microsoft Windows SDK. If you do not
Re: [HACKERS] [sepgsql 1/3] add name qualified creation label
John R Pierce writes: > On 1/23/2013 8:32 PM, Tom Lane wrote: >> FWIW, in Fedora-land I see: ... > I'd be far more interested in what is in RHEL and CentOS.Fedora, > with its 6 month obsolescence cycle, is of zero interest to me for > deploying database servers. But of course Fedora is also the upstream that will become RHEL7 and beyond. > EL6 has libselinux 2.0.94 > EL5 has libselinux 1.33.4 sepgsql already requires libselinux 2.0.99, so it doesn't appear to me that moving that goalpost is going to change things one way or the other for the existing RHEL branches. I couldn't ship contrib/sepgsql today in those branches. It might be that the update timing makes a bigger difference in some other distros, though. To return to Heikki's original point about Debian, what are they shipping today? 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] [sepgsql 1/3] add name qualified creation label
On 1/23/2013 8:32 PM, Tom Lane wrote: FWIW, in Fedora-land I see: F16: 2.1.6 (F16 will go out of support next month) F17: 2.1.10 (F17 has been stable for 6+ months) F18: 2.1.12 (F18 just went stable) While requiring 2.1.10 today might be thought a tad leading-edge, will that still be true by the time we ship 9.3? I'd be far more interested in what is in RHEL and CentOS.Fedora, with its 6 month obsolescence cycle, is of zero interest to me for deploying database servers. EL6 has libselinux 2.0.94 EL5 has libselinux 1.33.4 -- 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] [sepgsql 1/3] add name qualified creation label
Heikki Linnakangas writes: > On 17.01.2013 23:20, Kohei KaiGai wrote: >> In addition, I forgot to update minimum required version for libselinux; >> (it also takes change in configure script). > libselinux1 2.1.10 or newer is a pretty tall order. That's not in debian > testing yet, for example. I'm afraid if we bump the minimum requirement, > what might happen is that many distributions will stop building with > --with-selinux. FWIW, in Fedora-land I see: F16: 2.1.6 (F16 will go out of support next month) F17: 2.1.10 (F17 has been stable for 6+ months) F18: 2.1.12 (F18 just went stable) While requiring 2.1.10 today might be thought a tad leading-edge, will that still be true by the time we ship 9.3? Or maybe we should just be punting this patch to 9.4, by which time the version requirement should surely not be an issue within the community that's likely to be using selinux at all. Unless I missed a previous submission, this is a significant feature patch that did not show up in time for CF3, which means that we should not be considering it at all for 9.3 according to the rules that were agreed to in Ottawa last May. 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] Re: patch submission: truncate trailing nulls from heap rows to reduce the size of the null bitmap [Review]
Noah Misch writes: > On Wed, Jan 09, 2013 at 11:22:06AM +, Simon Riggs wrote: >> On balance, it would seem optimizing for this special case would >> affect everybody negatively; not much, but enough. Which means we >> should rekect this patch. > I've failed to come up with a non-arbitrary reason to recommend for or against > this patch, so I profess neutrality on the ultimate issue. I think if we can't convincingly show an attractive performance gain, we should reject the patch on the grounds of added complexity. Now, the amount of added code surely isn't much, but nonetheless this patch eliminates an invariant that's always been there (ie, that a tuple's natts matches the tupdesc it was built with). That will at the very least complicate forensic work, and it might well break third-party code, or corners of the core system that we haven't noticed yet. I'd be okay with this patch if it had more impressive performance results, but as it is I think we're better off without 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] pg_ctl idempotent option
I agree, answering the question, whether the particular attempt of starting a server succeeded or not, will need the current behaviour. Now, question is which of these behaviours should be default? Bruce, what if we make idempotent behaviour default and provide an option for current behaviour? On Thu, Jan 24, 2013 at 12:36 AM, Bruce Momjian wrote: > On Wed, Jan 23, 2013 at 09:00:25PM +0200, Heikki Linnakangas wrote: >> On 23.01.2013 20:56, Bruce Momjian wrote: >> >On Tue, Jan 22, 2013 at 06:03:28PM +0530, Ashutosh Bapat wrote: >> >>anyway, +1 for making this as default option. Going that path, would >> >>we be breaking backward compatibility? There might be scripts, (being >> >>already used), which depend upon the current behaviour. >> > >> >FYI, I have a pg_upgrade patch that relies on the old throw-an-error >> >behavior. Will there be a way to still throw an error if we make >> >idempotent the default? >> >> Could you check the status with "pg_ctl status" first, and throw an >> error if it's not what you expected? > > Well, this could still create a period of time where someone else starts > the server between my status and my starting it. Do we really want > that? And what if I want to start it with my special -o parameters, and > I then can't tell if it was already running or it is using my > parameters. I think an idempotent default is going to cause problems. > > -- > Bruce Momjian http://momjian.us > EnterpriseDB http://enterprisedb.com > > + It's impossible for everything to be true. + -- Best Wishes, Ashutosh Bapat EntepriseDB Corporation The Enterprise Postgres 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] Visual Studio 2012 RC
On 01/24/2013 09:38 AM, Noah Misch wrote: > The most notable difference is that I have no pre-VS2012 Microsoft > compilers installed and no SDKs installed by my explicit action. I > suggest assessing how the Framework64 directories get into your path > and trying without them. nm A further update on this: C:\Program Files (x86)\Microsoft Visual Studio 11.0\VC\vcvarsall.bat calls: C:\Program Files (x86)\Microsoft Visual Studio 11.0\VC\bin\vcvars32.bat which in turn runs: @call "%VS110COMNTOOLS%VCVarsQueryRegistry.bat" 32bit No64bit to start: C:\Program Files (x86)\Microsoft Visual Studio 11.0\Common7\Tools\VCVarsQueryRegistry.bat When I run this directly with the same arguments it adds to the environment: Framework35Version=v3.5 FrameworkDIR32=c:\Windows\Microsoft.NET\Framework64\ FrameworkVersion32=v4.0.30319 which is pretty clearly bogus. It looks like the script calls the subproc :GetFrameworkDir32Helper32 HKLM which does: reg query "HKLM\SOFTWARE\Microsoft\VisualStudio\SxS\VC7" /v "FrameworkDir32 resulting in: HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\VisualStudio\SxS\VC7 FrameworkDir32REG_SZc:\Windows\Microsoft.NET\Framework64\ which seems wrong. So it's clear that something's dodgy in how the various Microsoft tools have installed themselves, and it's nothing to do with your patch. Have you verified that 64-bit builds work? I'm testing now, but I've just confirmed that my machine isn't quite right. -- Craig Ringer 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] Visual Studio 2012 RC
On 01/24/2013 09:38 AM, Noah Misch wrote: > The most notable difference is that I have no pre-VS2012 Microsoft compilers installed and no SDKs installed by my explicit action. I suggest assessing how the Framework64 directories get into your path and trying without them. nm Interesting. The Framework64 directory is added to the PATH by vcvarsall.bat from VS 2012. I suspect what's happening is that VS assumes that if there's a Framework64 directory, it contains 64-bit compilers and tools from VS 2012. In my case, I have 64-bit tools from Windows SDK 7.1, but VS Express 2012 doesn't include a 64-bit toolset (only a cross-compiler) so the corresponding ones from 2012 aren't there. Alternately, the Windows SDK 8 installer may have installed the .NET Framework 4.5 64-bit components, and VS 2012 might not be expecting them. All this stuff is a terrifying pile of underdocumented hacks and mess upon hacks and mess, so it wouldn't be particularly surprising. If I manually prepend c:\Windows\Microsoft.NET\Framework\v4.0.30319 to the PATH I get a successful build and vcregress check passes. I'll just have a quick read of the patch but so far it looks good to me. -- Craig Ringer 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
[HACKERS] Re: patch submission: truncate trailing nulls from heap rows to reduce the size of the null bitmap [Review]
On Wed, Jan 09, 2013 at 11:22:06AM +, Simon Riggs wrote: > On 24 December 2012 16:57, Amit Kapila wrote: > > Performance: Average of 3 runs of pgbench in tps > > 9.3devel | with trailing null patch > > --+-- > > 578.9872 | 573.4980 > > On balance, it would seem optimizing for this special case would > affect everybody negatively; not much, but enough. Which means we > should rekect this patch. > > Do you have a reason why a different view might be taken? We've mostly ignored performance changes of a few percentage points, because the global consequences of adding or removing code to the binary image can easily change performance that much. It would be great to get to the point where we can reason about 1% performance changes, but we're not there. If a measured +1% performance change would have yielded a different decision, we should rethink the decision based on more-robust criteria. (Most of this was said in initial April 2012 discussion.) This patch is a tough one, because it will rarely help the most-common workloads. If it can reduce the tuple natts from 9 to 8, the tuple shrinks by a respectable eight bytes. But if it reduces natts from 72 to 9, we save nothing. It pays off nicely for the widest, sparsest tables: say, a table with 1000 columns, all but ten are NULL, and those non-NULL columns are near the front of the table. I've never seen such a design, but a few folks seemed to find it credible. I've failed to come up with a non-arbitrary reason to recommend for or against this patch, so I profess neutrality on the ultimate issue. Thanks, nm -- 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] Visual Studio 2012 RC
Hi Craig, Thanks for testing. On Wed, Jan 23, 2013 at 02:55:55PM +0800, Craig Ringer wrote: > When building a tree with your patch applied using VS 2012 Express via a > command line environment set up with: > >"c:\program files (x86)\Microsoft Visual Studio > 11.0\VC\vcvarsall.bat" x86 Likewise. > which is the same as the "32-bit build tools" Start menu entry, the > build fails with: > > > C:\Program Files > (x86)\MSBuild\Microsoft.Cpp\v4.0\Platforms\Win32\Microsoft.Cpp.Win32.Targets(518,5): > error MSB8008: Specified platform toolset (v110) is not installed or > invalid. Please make sure that a supported PlatformToolset value is > selected. > [c:\pg\postgresql\sdk8.0_cl17_vs11.0\x86\Release\vs2012\postgres.vcxproj] > > > In case it's relevant, the Microsoft SDK for Windows 8 is installed on > this machine and appears to have been detected, as WindowsSdkDir has > been set to C:\Program Files (x86)\Windows Kits\8.0\ . My WindowsSdkDir was the same. I had not manually installed the SDK; I suppose it was installed as part of Visual Studio Express 2012 for Windows Desktop. I tried manually installing Windows SDK 8.59.29750, and the build still worked fine. > I haven't explicitly set PlatformToolset in the environment. > > The machine also has Visual Studio 2010 Express SP1 and the Microsoft > Windows SDK 7.1 with VS SP1 and VS SP1 compiler update on it. All work fine. > > "where msbuild" reports: > > c:\Windows\Microsoft.NET\Framework64\v4.0.30319\MSBuild.exe > c:\Windows\Microsoft.NET\Framework64\v3.5\MSBuild.exe Mine are found in Framework, not Framework64: C:\Windows\Microsoft.NET\Framework\v4.0.30319\MSBuild.exe C:\Windows\Microsoft.NET\Framework\v3.5\MSBuild.exe If I prepend Framework64 to my path, the build does fail, albeit not the same way your build fails: d:\cygwin\home\nm\src\pg\postgresql\postgres.vcxproj(16,3): error MSB4019: The imported project "d:\Microsoft.Cpp.Default.props" was not found. Confirm that the path in the declaration is correct, and that the file exists on disk. > and "msbuild" reports: > > Microsoft (R) Build Engine version 4.0.30319.17929 > [Microsoft .NET Framework, version 4.0.30319.17929] > Copyright (C) Microsoft Corporation. All rights reserved. Identical: Microsoft (R) Build Engine version 4.0.30319.17929 [Microsoft .NET Framework, version 4.0.30319.17929] Copyright (C) Microsoft Corporation. All rights reserved. > "cl" reports: > > Microsoft (R) C/C++ Optimizing Compiler Version 17.00.50727.1 for x86 > Copyright (C) Microsoft Corporation. All rights reserved. > > usage: cl [ option... ] filename... [ /link linkoption... ] Identical: Microsoft (R) C/C++ Optimizing Compiler Version 17.00.50727.1 for x86 Copyright (C) Microsoft Corporation. All rights reserved. > The host is a 64-bit Windows 7 machine. Windows Server 2008 R2, 64-bit, SP1 > How have you been testing VS2012 builds? In what environment? The most notable difference is that I have no pre-VS2012 Microsoft compilers installed and no SDKs installed by my explicit action. I suggest assessing how the Framework64 directories get into your path and trying without them. nm -- 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] logical changeset generation v4 - Heikki's thoughts about the patch state
On 01/23/2013 05:17 PM, Robert Haas wrote: Of course, I have no evidence that that will happen. But it is a really big piece of code, and therefore unless you are superman, it's probably got a really large number of bugs. The scary thing is that it is not as if we can say, well, this is a big hunk of code, but it doesn't really touch the core of the system, so if it's broken, it'll be broken itself, but it won't break anything else. Rather, this code is deeply in bed with WAL, with MVCC, and with the on-disk format of tuples, and makes fundamental changes to the first two of those. You agreed with Tom that 9.2 is the buggiest release in recent memory, but I think logical replication could easily be an order of magnitude worse. Command Prompt worked for YEARS to get logical replication right and we never got it to the point where I would have been happy submitting it to -core. It behooves .Org to be extremely conservative about this feature. Granted, it is a feature we should have had years ago but still. It is not a simple thing, it is not an easy thing. It is complicated and complex to get correcft. JD -- Command Prompt, Inc. - http://www.commandprompt.com/ PostgreSQL Support, Training, Professional Services and Development High Availability, Oracle Conversion, Postgres-XC @cmdpromptinc - 509-416-6579 -- 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] logical changeset generation v4 - Heikki's thoughts about the patch state
On Wed, Jan 23, 2013 at 5:30 PM, Andres Freund wrote: > pgbench upstream: > tps: 22275.941409 > space overhead: 0% > pgbench logical-submitted > tps: 16274.603046 > space overhead: 2.1% > pgbench logical-HEAD (will submit updated version tomorrow or so): > tps: 20853.341551 > space overhead: 2.3% > pgbench single plpgsql trigger (INSERT INTO log(data) VALUES(NEW::text)) > tps: 14101.349535 > space overhead: 369% > > Note that in the single trigger case nobody consumed the queue while the > logical version streamed the changes out and stored them to disk. > > Adding a default NOW() or similar to the tables immediately makes > logical decoding faster by a factor of about 3 in comparison to the > above trivial trigger. > > The only reason the submitted version of logical decoding is > comparatively slow is that its xmin update policy is braindamaged, > working on that right now. I agree. The thing that scares me about the logical replication stuff is not that it might be slow (and if your numbers are to be believed, it isn't), but that I suspect it's riddled with bugs and possibly some questionable design decisions. If we commit it and release it, then we're going to be stuck maintaining it for a very, very long time. If it turns out to have serious bugs that can't be fixed without a new major release, it's going to be a serious black eye for the project. Of course, I have no evidence that that will happen. But it is a really big piece of code, and therefore unless you are superman, it's probably got a really large number of bugs. The scary thing is that it is not as if we can say, well, this is a big hunk of code, but it doesn't really touch the core of the system, so if it's broken, it'll be broken itself, but it won't break anything else. Rather, this code is deeply in bed with WAL, with MVCC, and with the on-disk format of tuples, and makes fundamental changes to the first two of those. You agreed with Tom that 9.2 is the buggiest release in recent memory, but I think logical replication could easily be an order of magnitude worse. I also have serious concerns about checksums and foreign key locks. Any single one of those three patches could really inflict unprecedented damage on our community's reputation for stability and reliability if they turn out to be seriously buggy, and unfortunately I don't consider that an unlikely outcome. I don't know what to do about it, either. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pg_isready (was: [WIP] pg_ping utility)
On Thu, Jan 24, 2013 at 8:47 AM, Phil Sorber wrote: > On Wed, Jan 23, 2013 at 6:07 PM, Tom Lane wrote: >> Phil Sorber writes: >>> On Wed, Jan 23, 2013 at 1:58 PM, Bruce Momjian wrote: On Wed, Jan 23, 2013 at 12:27:45PM -0500, Tom Lane wrote: > +1 for default timeout --- if this isn't like "ping" where you are > expecting to run indefinitely, I can't see that it's a good idea for it > to sit very long by default, in any circumstance. >> FYI, the pg_ctl -w (wait) default is 60 seconds: >> >>> Great. That is what I came to on my own as well. Figured that might be >>> a sticking point, but as there is precedent, I'm happy with it. >> >> I'm not sure that's a relevant precedent at all. What that number is >> is the time that pg_ctl will wait around for the postmaster to start or >> stop before reporting a problem --- and in either case, a significant >> delay (multiple seconds) is not surprising, because of crash-recovery >> work, shutdown checkpointing, etc. For pg_isready, you'd expect to get >> a response more or less instantly, wouldn't you? Personally, I'd decide >> that pg_isready is broken if it didn't give me an answer in a couple of >> seconds, much less a minute. >> >> What I had in mind was a default timeout of maybe 3 or 4 seconds... > > I was thinking that if it was in a script you wouldn't care if it was > 60 seconds. If it was at the command line you would ^C it much > earlier. I think the default linux TCP connection timeout is around 20 > seconds. My feeling is everyone is going to have a differing opinion > on this, which is why I was hoping that some good precedent existed. > I'm fine with 3 or 4, whatever can be agreed upon. +1 with 3 or 4 secounds. Aside from this issue, I have one minor comment. ISTM you need to add the following line to the end of the help message. This line has been included in the help message of all the other client commands. Report bugs to . 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] Back-branch update releases coming in a couple weeks
On Thu, Jan 24, 2013 at 7:42 AM, MauMau wrote: > From: "Tom Lane" > >> Since we've fixed a couple of relatively nasty bugs recently, the core >> committee has determined that it'd be a good idea to push out PG update >> releases soon. The current plan is to wrap on Monday Feb 4 for public >> announcement Thursday Feb 7. If you're aware of any bug fixes you think >> ought to get included, now's the time to get them done ... > > > I've just encountered a serious problem, and I really wish it would be fixed > in the upcoming minor release. Could you tell me whether this is already > fixed and will be included? > > I'm using synchronous streaming replication with PostgreSQL 9.1.6 on Linux. > There are two nodes: one is primary and the other is a standby. When I > performed failover tests by doing "pg_ctl stop -mi" against the primary > while some applications are reading/writing the database, the standby > crashed while it was performing recovery after receiving promote request: > > ... > LOG: archive recovery complete > WARNING: page 506747 of relation base/482272/482304 was uninitialized > PANIC: WAL contains references to invalid pages > LOG: startup process (PID 8938) was terminated by signal 6: Aborted > LOG: terminating any other active server processes > (the log ends here) > > The mentioned relation is an index. The contents of the referred page was > all zeros when I looked at it with "od -t x $PGDATA/base/482272/482304.3" > after the crash. The subsequent pages of the same file had valid-seeming > contents. > > I searched through PostgreSQL mailing lists with "WAL contains references to > invalid pages", and i found 19 messages. Some people encountered similar > problem. There were some discussions regarding those problems (Tom and > Simon Riggs commented), but those discussions did not reach a solution. > > I also found a discussion which might relate to this problem. Does this fix > the problem? > > [BUG] lag of minRecoveryPont in archive recovery > http://www.postgresql.org/message-id/20121206.130458.170549097.horiguchi.kyot...@lab.ntt.co.jp Yes. Could you check whether you can reproduce the problem on the latest REL9_2_STABLE? 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] Visual Studio 2012 RC
On 01/24/2013 03:23 AM, Brar Piening wrote: >> On 01/23/2013 02:14 PM, Craig Ringer wrote: >> >> How have you been testing VS2012 builds? In what environment? > > When I tested this patch the last time I've been using Windows 8 RTM > (Microsoft Windows 8 Enterprise Evaluation - 6.2.9200 Build 9200) and > Microsoft Visual Studio Express 2012 für Windows Desktop RTM (Version > 11.0.50727.42) ... and how are you setting up your build environment? Can you show the steps you're following to get a successful build using this patch? If you install the Windows 8 SDK, do you encounter issues? -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Re: [HACKERS] [PATCH] pg_isready (was: [WIP] pg_ping utility)
On Wed, Jan 23, 2013 at 6:07 PM, Tom Lane wrote: > Phil Sorber writes: >> On Wed, Jan 23, 2013 at 1:58 PM, Bruce Momjian wrote: >>> On Wed, Jan 23, 2013 at 12:27:45PM -0500, Tom Lane wrote: +1 for default timeout --- if this isn't like "ping" where you are expecting to run indefinitely, I can't see that it's a good idea for it to sit very long by default, in any circumstance. > >>> FYI, the pg_ctl -w (wait) default is 60 seconds: > >> Great. That is what I came to on my own as well. Figured that might be >> a sticking point, but as there is precedent, I'm happy with it. > > I'm not sure that's a relevant precedent at all. What that number is > is the time that pg_ctl will wait around for the postmaster to start or > stop before reporting a problem --- and in either case, a significant > delay (multiple seconds) is not surprising, because of crash-recovery > work, shutdown checkpointing, etc. For pg_isready, you'd expect to get > a response more or less instantly, wouldn't you? Personally, I'd decide > that pg_isready is broken if it didn't give me an answer in a couple of > seconds, much less a minute. > > What I had in mind was a default timeout of maybe 3 or 4 seconds... I was thinking that if it was in a script you wouldn't care if it was 60 seconds. If it was at the command line you would ^C it much earlier. I think the default linux TCP connection timeout is around 20 seconds. My feeling is everyone is going to have a differing opinion on this, which is why I was hoping that some good precedent existed. I'm fine with 3 or 4, whatever can be agreed upon. > > 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] [COMMITTERS] pgsql: Improve concurrency of foreign key locking
Simon Riggs writes: > On 23 January 2013 17:15, Andres Freund wrote: >> On 2013-01-23 11:58:28 -0500, Andrew Dunstan wrote: >>> Can't we do better than that? > "row level locks cannot be applied to the NULLable side of an outer join" I think it should read "row-level locks cannot ...", but otherwise this is a fine idea. > Hint: there are no rows to lock This bit doesn't seem to be either accurate or helpful, though. 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] pg_isready (was: [WIP] pg_ping utility)
Phil Sorber writes: > On Wed, Jan 23, 2013 at 1:58 PM, Bruce Momjian wrote: >> On Wed, Jan 23, 2013 at 12:27:45PM -0500, Tom Lane wrote: >>> +1 for default timeout --- if this isn't like "ping" where you are >>> expecting to run indefinitely, I can't see that it's a good idea for it >>> to sit very long by default, in any circumstance. >> FYI, the pg_ctl -w (wait) default is 60 seconds: > Great. That is what I came to on my own as well. Figured that might be > a sticking point, but as there is precedent, I'm happy with it. I'm not sure that's a relevant precedent at all. What that number is is the time that pg_ctl will wait around for the postmaster to start or stop before reporting a problem --- and in either case, a significant delay (multiple seconds) is not surprising, because of crash-recovery work, shutdown checkpointing, etc. For pg_isready, you'd expect to get a response more or less instantly, wouldn't you? Personally, I'd decide that pg_isready is broken if it didn't give me an answer in a couple of seconds, much less a minute. What I had in mind was a default timeout of maybe 3 or 4 seconds... 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] Back-branch update releases coming in a couple weeks
From: "Tom Lane" Since we've fixed a couple of relatively nasty bugs recently, the core committee has determined that it'd be a good idea to push out PG update releases soon. The current plan is to wrap on Monday Feb 4 for public announcement Thursday Feb 7. If you're aware of any bug fixes you think ought to get included, now's the time to get them done ... I've just encountered a serious problem, and I really wish it would be fixed in the upcoming minor release. Could you tell me whether this is already fixed and will be included? I'm using synchronous streaming replication with PostgreSQL 9.1.6 on Linux. There are two nodes: one is primary and the other is a standby. When I performed failover tests by doing "pg_ctl stop -mi" against the primary while some applications are reading/writing the database, the standby crashed while it was performing recovery after receiving promote request: ... LOG: archive recovery complete WARNING: page 506747 of relation base/482272/482304 was uninitialized PANIC: WAL contains references to invalid pages LOG: startup process (PID 8938) was terminated by signal 6: Aborted LOG: terminating any other active server processes (the log ends here) The mentioned relation is an index. The contents of the referred page was all zeros when I looked at it with "od -t x $PGDATA/base/482272/482304.3" after the crash. The subsequent pages of the same file had valid-seeming contents. I searched through PostgreSQL mailing lists with "WAL contains references to invalid pages", and i found 19 messages. Some people encountered similar problem. There were some discussions regarding those problems (Tom and Simon Riggs commented), but those discussions did not reach a solution. I also found a discussion which might relate to this problem. Does this fix the problem? [BUG] lag of minRecoveryPont in archive recovery http://www.postgresql.org/message-id/20121206.130458.170549097.horiguchi.kyot...@lab.ntt.co.jp Regards MauMau -- 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] logical changeset generation v4 - Heikki's thoughts about the patch state
Hi, I decided to reply on the patches thread to be able to find this later. On 2013-01-23 22:48:50 +0200, Heikki Linnakangas wrote: > "logical changeset generation v4" > This is a boatload of infrastructure for supporting logical replication, yet > we have no code actually implementing logical replication that would go with > this. The premise of logical replication over trigger-based was that it'd be > faster, yet we cannot asses that without a working implementation. I don't > think this can be committed in this state. Its a fair point that this is a huge amount of code without a user in itself in-core. But the reason it got no user included is because several people explicitly didn't want a user in-core for now but said the first part of this would be to implement the changeset generation as a separate piece. Didn't you actually prefer not to have any users of this in-core yourself? Also, while the apply side surely isn't benchmarkable without any being submitted, the changeset generation can very well be benchmarked. A very, very adhoc benchmark: -c max_wal_senders=10 -c max_logical_slots=10 --disabled for anything but logical -c wal_level=logical --hot_standby for anything but logical -c checkpoint_segments=100 -c log_checkpoints=on -c shared_buffers=512MB -c autovacuum=on -c log_min_messages=notice -c log_line_prefix='[%p %t] ' -c wal_keep_segments=100 -c fsync=off -c synchronous_commit=off pgbench -p 5440 -h /tmp -n -M prepared -c 16 -j 16 -T 30 pgbench upstream: tps: 22275.941409 space overhead: 0% pgbench logical-submitted tps: 16274.603046 space overhead: 2.1% pgbench logical-HEAD (will submit updated version tomorrow or so): tps: 20853.341551 space overhead: 2.3% pgbench single plpgsql trigger (INSERT INTO log(data) VALUES(NEW::text)) tps: 14101.349535 space overhead: 369% Note that in the single trigger case nobody consumed the queue while the logical version streamed the changes out and stored them to disk. Adding a default NOW() or similar to the tables immediately makes logical decoding faster by a factor of about 3 in comparison to the above trivial trigger. The only reason the submitted version of logical decoding is comparatively slow is that its xmin update policy is braindamaged, working on that right now. Greetings, Andres -- 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
[HACKERS] Re: [BUGS] BUG #7815: Upgrading PostgreSQL from 9.1 to 9.2 with pg_upgrade/postgreql-setup fails - invalid status retrieve
On Sat, Jan 19, 2013 at 09:56:48PM -0500, Bruce Momjian wrote:
> > (But, at least with the type of packaging I'm using in Fedora, he would
> > first have to go through a package downgrade/reinstallation process,
> > because the packaging provides no simple scripted way of manually
> > starting the old postgres executable, only the new one. Moreover, what
> > pg_upgrade is printing provides no help in figuring out whether that's
> > the next step.)
> >
> > I do sympathize with taking a paranoid attitude here, but I'm failing
> > to see what advantage there is in not attempting to start the old
> > postmaster. In the *only* case that pg_upgrade is successfully
> > protecting against with this logic, namely there's-an-active-postmaster-
> > already, the postmaster is equally able to protect itself. In other
> > cases it would be more helpful not less to let the postmaster analyze
> > the situation.
> >
> > > The other problem is that if the server start fails, how do we know if
> > > the failure was due to a running postmaster?
> >
> > Because we read the postmaster's log file, or at least tell the user to
> > do so. That report would be unambiguous, unlike pg_upgrade's.
>
> Attached is a WIP patch to give you an idea of how I am going to solve
> this problem. This comment says it all:
Attached is a ready-to-apply version of the patch. I continued to use
postmaster.pid to determine if I need to try the start/stop test ---
that allows me to know which servers _might_ be running, because a
server start failure might be for many reasons and I would prefer not to
suggest the server is running if I can avoid it, and the pid file gives
me that.
The patch is longer than I expected because the code needed to be
reordered. The starting banner indicates if it a live check, but to
check for a live server we need to start/stop the servers and we needed
to know where the binaries are, and we didn't do that until after the
start banner. I removed the 'check' line for binary checks, and moved
that before the banner printing. postmaster_start also now needs an
option to supress an error.
--
Bruce Momjian http://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
diff --git a/contrib/pg_upgrade/check.c b/contrib/pg_upgrade/check.c
new file mode 100644
index 1780788..8188643
*** a/contrib/pg_upgrade/check.c
--- b/contrib/pg_upgrade/check.c
*** fix_path_separator(char *path)
*** 56,66
}
void
! output_check_banner(bool *live_check)
{
! if (user_opts.check && is_server_running(old_cluster.pgdata))
{
- *live_check = true;
pg_log(PG_REPORT, "Performing Consistency Checks on Old Live Server\n");
pg_log(PG_REPORT, "\n");
}
--- 56,65
}
void
! output_check_banner(bool live_check)
{
! if (user_opts.check && live_check)
{
pg_log(PG_REPORT, "Performing Consistency Checks on Old Live Server\n");
pg_log(PG_REPORT, "\n");
}
*** check_and_dump_old_cluster(bool live_che
*** 78,84
/* -- OLD -- */
if (!live_check)
! start_postmaster(&old_cluster);
set_locale_and_encoding(&old_cluster);
--- 77,83
/* -- OLD -- */
if (!live_check)
! start_postmaster(&old_cluster, true);
set_locale_and_encoding(&old_cluster);
*** issue_warnings(char *sequence_script_fil
*** 201,207
/* old = PG 8.3 warnings? */
if (GET_MAJOR_VERSION(old_cluster.major_version) <= 803)
{
! start_postmaster(&new_cluster);
/* restore proper sequence values using file created from old server */
if (sequence_script_file_name)
--- 200,206
/* old = PG 8.3 warnings? */
if (GET_MAJOR_VERSION(old_cluster.major_version) <= 803)
{
! start_postmaster(&new_cluster, true);
/* restore proper sequence values using file created from old server */
if (sequence_script_file_name)
*** issue_warnings(char *sequence_script_fil
*** 224,230
/* Create dummy large object permissions for old < PG 9.0? */
if (GET_MAJOR_VERSION(old_cluster.major_version) <= 804)
{
! start_postmaster(&new_cluster);
new_9_0_populate_pg_largeobject_metadata(&new_cluster, false);
stop_postmaster(false);
}
--- 223,229
/* Create dummy large object permissions for old < PG 9.0? */
if (GET_MAJOR_VERSION(old_cluster.major_version) <= 804)
{
! start_postmaster(&new_cluster, true);
new_9_0_populate_pg_largeobject_metadata(&new_cluster, false);
stop_postmaster(false);
}
diff --git a/contrib/pg_upgrade/exec.c b/contrib/pg_upgrade/exec.c
new file mode 100644
index e326a10..44dafc3
*** a/contrib/pg_upgrade/exec.c
--- b/contrib/pg_upgrade/exec.c
*** exec_prog(const char *log_file, const ch
*** 140,152
/*
! * is_server_running()
*
! * checks whether postmaster o
Re: [HACKERS] CF3+4 (was Re: Parallel query execution)
Hello > > I do that pretty often. A better approach, imv, would be making psql a > bit more of a 'real' shell, with loops, conditionals, better variable > handling, etc. > after a few years prototyping on this area I am not sure so this is good idea. Maybe better to start some new console from scratch. >> "plpgsql_check_function" >> I don't like this in its current form. A lot of code that mirrors >> pl_exec.c. I think we'll have to find a way to somehow re-use the >> existing code for this. Like, compile the function as usual, but >> don't stop on error. > > Havn't looked at this yet, but your concerns make sense to me. I invite any moving in this subject - again I wrote lot of variants and current is a most maintainable (my opinion) - there is redundant structure (not code) - and simply code. After merging the code lost readability :(. But I would to continue in work - I am sure so it has a sense and people and some large companies use it with success, so it should be in core - in any form. Regards Pavel > > Thanks! > > Stephen -- 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] Potential TODO: schema in ALTER DEFAULT PRIVILEGES?
Josh, * Josh Berkus (j...@agliodbs.com) wrote: > As you know, there's a lot of people these days using SCHEMA for > multi-tenant application partitioning. One of them pointed out to me > that "schema" is missing from ALTER DEFAULT PRIVS; that is, there's no > way for you to set default permissions on a new schema. For folks using > schema for partitioning, support for this would be very helpful. > > Worth adding to TODO? Obviously nobody's going to work on it right now. The original ALTER DEFAULT PRIVS actually included support for exactly this, and there was a patch at one point for DEFAULT OWNER as well. I'm on board for both of those ideas and run into the lack of them regularly (as in, last week I was setting default privileges for a whole slew of roles by hand for a given schema because I couldn't set it for *all* users for a given schema, even as a superuser, and new roles will be added shortly and I'll have to go back and remember to add the default privs for them also...). That's my 2c. I don't believe this is really a question about if anyone needs this so much as how we can implement it and keep everyone happy that it's safe and secure. That's what needs to be worked out first. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] CF3+4 (was Re: Parallel query execution)
Heikki, * Heikki Linnakangas (hlinnakan...@vmware.com) wrote: > FWIW, here's how I feel about some the patches. It's not an exhaustive list. Thanks for going through them and commenting on them. > "Event Triggers: Passing Information to User Functions (from 2012-11)" > I don't care about this whole feature, and haven't looked at the > patch.. Probably not worth the complexity. But won't object if > someone else wants to deal with it. I'd like to see it happen. > "Extension templates" > Same as above. This one isn't actually all that complex, though it does add a few additional catalogs for keeping track of things. For my part, I'd like to see this go in as I see it being another step closer to Packages that a certain other RDBMS has. > "Checksums (initdb-time)" > I don't want this feature, and I've said that on the thread. I see a lot of value in this. > "Identity projection (partitioning)" > Nothing's happened for over a month. Seems dead for that reason. I don't think this is dead.. > "Remove unused targets from plan" > Same here. > > "Store additional information in GIN index" > Same here. Havn't been following these so not sure. Some of these are in a state of lack of progress for having not been reviewed. > "Index based regexp search for pg_trgm" > I'd like to see this patch go in, but it still needs a fair amount > of work. Probably needs to be pushed to next commitfest for that > reason. Agreed. > "allowing privileges on untrusted languages" > Seems like a bad idea to me, for the reasons Tom mentioned on that thread. On the fence about this one. I like the idea of reducing the need to be a superuser, but there are risks associated with this change also. > "Skip checkpoint on promoting from streaming replication" > Given that we still don't have an updated patch for this, it's > probably getting too late for this. Would be nice to see the patch > or an explanation of the new design Simon's been working. > > "Patch to compute Max LSN of Data Pages (from 2012-11)" > Meh. Seems like a really special-purpose tool. Probably better to > put this on pgfoundry. Agreed on these two. > "logical changeset generation v4" > This is a boatload of infrastructure for supporting logical > replication, yet we have no code actually implementing logical > replication that would go with this. The premise of logical > replication over trigger-based was that it'd be faster, yet we > cannot asses that without a working implementation. I don't think > this can be committed in this state. Andres had a working implementation of logical replication, with code to back it up and performance numbers showing how much faster it is, at PGCon last year, as I recall... Admittedly, it probably needs changing and updating due to the changes which the patches have been going through, but I don't think the claim that we don't know it's faster is fair. > "built-in/SQL Command to edit the server configuration file > (postgresql.conf)" > I think this should be a pgfoundry project, not in core. At least for now. I don't think it would ever get deployed or used then.. > "json generator function enhacements" > "Json API and extraction functions" > To be honest, I don't understand why the json datatype had to be > built-in to begin with. But it's too late to object to that now, and > if the datatype is there, these functions probably should be, too. > Or could these be put into a separate "json-extras" extension? I > dunno. There were good reasons to add json as a data type, I thought, though I don't have them offhand. > "psql watch" > Meh. In practice, for the kind of ad-hoc monitoring this would be > useful for, you might as well just use "watch psql -c 'select ...' > ". Yes, that reconnects for each query, but so what. I do that pretty often. A better approach, imv, would be making psql a bit more of a 'real' shell, with loops, conditionals, better variable handling, etc. > "plpgsql_check_function" > I don't like this in its current form. A lot of code that mirrors > pl_exec.c. I think we'll have to find a way to somehow re-use the > existing code for this. Like, compile the function as usual, but > don't stop on error. Havn't looked at this yet, but your concerns make sense to me. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] [sepgsql 1/3] add name qualified creation label
On 17.01.2013 23:20, Kohei KaiGai wrote: 2013/1/16 Robert Haas: This looks OK on a quick once-over, but should it update the documentation somehow? Documentation does not take so much description for type_transition rules, so I just modified relevant description a bit to mention about type_transition rule may have argument of new object name optionally. The comments at least need updating, to mention the new arguments. In addition, I forgot to update minimum required version for libselinux; (it also takes change in configure script). libselinux1 2.1.10 or newer is a pretty tall order. That's not in debian testing yet, for example. I'm afraid if we bump the minimum requirement, what might happen is that many distributions will stop building with --with-selinux. - 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] [COMMITTERS] pgsql: Improve concurrency of foreign key locking
On 01/23/2013 12:48 PM, Simon Riggs wrote:
On 23 January 2013 17:15, Andres Freund wrote:
On 2013-01-23 11:58:28 -0500, Andrew Dunstan wrote:
On 01/23/2013 10:12 AM, Alvaro Herrera wrote:
Improve concurrency of foreign key locking
This error message change looks rather odd, and has my head spinning a bit:
-errmsg("SELECT FOR UPDATE/SHARE cannot be applied to
the nullable side of an outer join")));
+errmsg("SELECT FOR UPDATE/SHARE/KEY UPDATE/KEY SHARE
cannot be applied to the nullable side of an outer join")))
Can't we do better than that?
I don't really see how? I don't think listing only the current locklevel
really is an improvement and something like "SELECT ... FOR $locktype
cannot .." seem uncommon enough in pg error messages to be strange.
Now I aggree that listing all those locklevels isn't that nice, but I
don't really have a better idea.
"row level locks cannot be applied to the NULLable side of an outer join"
Hint: there are no rows to lock
Yeah, this is really more informative than either, I think.
cheers
andrew
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Potential TODO: schema in ALTER DEFAULT PRIVILEGES?
Folks, As you know, there's a lot of people these days using SCHEMA for multi-tenant application partitioning. One of them pointed out to me that "schema" is missing from ALTER DEFAULT PRIVS; that is, there's no way for you to set default permissions on a new schema. For folks using schema for partitioning, support for this would be very helpful. Worth adding to TODO? Obviously nobody's going to work on it right now. -- 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] CF3+4 (was Re: Parallel query execution)
On 23.01.2013 20:44, Stephen Frost wrote: * Robert Haas (robertmh...@gmail.com) wrote: For all of that, I'm not sure that people failing to seek consensus before coding is really so much of a problem as you seem to think. For my part, I don't think the lack of consensus-finding before submitting patches is, in itself, a problem. I feel the same. Posting a patch before design and consensus may be a huge waste of time for the submitter himself, but it's not a problem for others. The problem, imv, is that everyone is expecting that once they've written a patch and put it on a commitfest that it's going to get committed- and it seems like committers are feeling under pressure that, because something's on the CF app, it needs to be committed in some form. I'm sure none of the committers have a problem rejecting a patch, when there's clear grounds for it. Rejecting is the easiest way to deal with a patch. However, at least for me, >50% of the patches in any given commitfest don't interest me at all. I don't object to them, per se, but I don't want to spend any time on them either. I can imagine the same to be true for all other committers too. If a patch doesn't catch the interest of any committer, it's going to just sit there in the commitfest app for a long time, even if it's been reviewed. As discussed, we really need to be ready to truely triage the remaining patch set, figure out who is going to work on what, and punt the rest til post-9.3. FWIW, here's how I feel about some the patches. It's not an exhaustive list. "Event Triggers: Passing Information to User Functions (from 2012-11)" I don't care about this whole feature, and haven't looked at the patch.. Probably not worth the complexity. But won't object if someone else wants to deal with it. "Extension templates" Same as above. "Checksums (initdb-time)" I don't want this feature, and I've said that on the thread. "Identity projection (partitioning)" Nothing's happened for over a month. Seems dead for that reason. "Remove unused targets from plan" Same here. "Store additional information in GIN index" Same here. "Index based regexp search for pg_trgm" I'd like to see this patch go in, but it still needs a fair amount of work. Probably needs to be pushed to next commitfest for that reason. "allowing privileges on untrusted languages" Seems like a bad idea to me, for the reasons Tom mentioned on that thread. "Skip checkpoint on promoting from streaming replication" Given that we still don't have an updated patch for this, it's probably getting too late for this. Would be nice to see the patch or an explanation of the new design Simon's been working. "Patch to compute Max LSN of Data Pages (from 2012-11)" Meh. Seems like a really special-purpose tool. Probably better to put this on pgfoundry. "logical changeset generation v4" This is a boatload of infrastructure for supporting logical replication, yet we have no code actually implementing logical replication that would go with this. The premise of logical replication over trigger-based was that it'd be faster, yet we cannot asses that without a working implementation. I don't think this can be committed in this state. "built-in/SQL Command to edit the server configuration file (postgresql.conf)" I think this should be a pgfoundry project, not in core. At least for now. "json generator function enhacements" "Json API and extraction functions" To be honest, I don't understand why the json datatype had to be built-in to begin with. But it's too late to object to that now, and if the datatype is there, these functions probably should be, too. Or could these be put into a separate "json-extras" extension? I dunno. "psql watch" Meh. In practice, for the kind of ad-hoc monitoring this would be useful for, you might as well just use "watch psql -c 'select ...' ". Yes, that reconnects for each query, but so what. "plpgsql_check_function" I don't like this in its current form. A lot of code that mirrors pl_exec.c. I think we'll have to find a way to somehow re-use the existing code for this. Like, compile the function as usual, but don't stop on error. - 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] CF3+4 (was Re: Parallel query execution)
On Wed, Jan 23, 2013 at 3:23 PM, Phil Sorber wrote: > On Wed, Jan 23, 2013 at 1:44 PM, Stephen Frost wrote: >> * Robert Haas (robertmh...@gmail.com) wrote: >>> For all of that, I'm not sure that people failing to seek consensus >>> before coding is really so much of a problem as you seem to think. >> >> For my part, I don't think the lack of consensus-finding before >> submitting patches is, in itself, a problem. >> >> The problem, imv, is that everyone is expecting that once they've >> written a patch and put it on a commitfest that it's going to get >> committed- and it seems like committers are feeling under pressure >> that, because something's on the CF app, it needs to be committed >> in some form. >> > > FWIW, I have NO delusions that something I propose or submit or put in > a CF is necessarily going to get committed. For me it's not committed > until I can see it in 'git log' and even then, I've seen stuff get > reverted. I would hope that if a committer isn't comfortable with a > patch they would explain why, and decline to commit. Then it's up to > the submitter as to whether or not they want to make changes, try to > explain why they are right and the committer is wrong, or withdraw the > patch. I think that's the right attitude, but it doesn't always work out that way. Reviewers and committers sometimes spend a lot of time writing a review and then get flamed for their honest opinion about the readiness of a patch. Of course, reviewers and committers can be jerks, too. As far as I know, we're all human, here. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: Proposal for Allow postgresql.conf values to be changed via SQL [review]
On 2013-01-22 12:32:07 +, Amit kapila wrote: > This closes all comments raised till now for this patch. > Kindly let me know if you feel something is missing? I am coming late to this patch, so bear with me if I repeat somethign said elsewhere. Review comments of cursory pass through the patch: * most comments are hard to understand. I know the problem of that being hard for a non-native speaker by heart, but I think another pass over them would be good thing. * The gram.y changes arround set_rest_(more|common) seem pretty confused to me. E.g. its not possible anymore to set the timezone for a function. And why is it possible to persistently set the search path, but not client encoding? Why is FROM CURRENT in set_rest_more? * set_config_file should elog(ERROR), not return on an unhandled setstmt->kind * why are you creating AutoConfFileName if its not stat'able? It seems better to simply skip parsing the old file in that case * Writing the temporary file to .$pid seems like a bad idea, better use one file for that, SET PERSISTENT is protected by an exclusive lock anyway. * the write sequence should be: * fsync(tempfile) * fsync(directory) * rename(tempfile, configfile) * fsync(configfile) * fsync(directory) * write_auto_conf_file should probably escape quoted values? * coding style should be adhered to more closesly, there are many if (pointer) which should be if (pointer != NULL), single-line blocks enclosed in curlies which shouldn't, etc. * replace_auto_config_file and surrounding functions need more comments in the header * the check that prevents persistent SETs in a transaction should rather be in utility.c and use PreventTransactionChain like most of the others that need to do that (c.f. T_CreatedbStmt). I think this patch is a good bit away of being ready for committer... 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] CF3+4 (was Re: Parallel query execution)
On Wed, Jan 23, 2013 at 1:44 PM, Stephen Frost wrote: > * Robert Haas (robertmh...@gmail.com) wrote: >> For all of that, I'm not sure that people failing to seek consensus >> before coding is really so much of a problem as you seem to think. > > For my part, I don't think the lack of consensus-finding before > submitting patches is, in itself, a problem. > > The problem, imv, is that everyone is expecting that once they've > written a patch and put it on a commitfest that it's going to get > committed- and it seems like committers are feeling under pressure > that, because something's on the CF app, it needs to be committed > in some form. > FWIW, I have NO delusions that something I propose or submit or put in a CF is necessarily going to get committed. For me it's not committed until I can see it in 'git log' and even then, I've seen stuff get reverted. I would hope that if a committer isn't comfortable with a patch they would explain why, and decline to commit. Then it's up to the submitter as to whether or not they want to make changes, try to explain why they are right and the committer is wrong, or withdraw the patch. > There's a lot of good stuff out there, sure, and even more good *ideas*, > but it's important to make sure we can provide a stable system with > regular releases. As discussed, we really need to be ready to truely > triage the remaining patch set, figure out who is going to work on what, > and punt the rest til post-9.3. > > Thanks, > > Stephen -- 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] pg_isready (was: [WIP] pg_ping utility)
On Wed, Jan 23, 2013 at 2:51 PM, Bruce Momjian wrote: > On Wed, Jan 23, 2013 at 02:50:01PM -0500, Phil Sorber wrote: >> On Wed, Jan 23, 2013 at 1:58 PM, Bruce Momjian wrote: >> > On Wed, Jan 23, 2013 at 12:27:45PM -0500, Tom Lane wrote: >> >> Phil Sorber writes: >> >> > On Wed, Jan 23, 2013 at 11:07 AM, Robert Haas >> >> > wrote: >> >> >> [rhaas pgsql]$ pg_isready -h www.google.com >> >> >> >> >> >> >> > Do you think we should have a default timeout, or only have one if >> >> > specified at the command line? >> >> >> >> +1 for default timeout --- if this isn't like "ping" where you are >> >> expecting to run indefinitely, I can't see that it's a good idea for it >> >> to sit very long by default, in any circumstance. >> > >> > FYI, the pg_ctl -w (wait) default is 60 seconds: >> > >> > from pg_ctl.c: >> > >> > #define DEFAULT_WAIT60 >> > >> >> Great. That is what I came to on my own as well. Figured that might be >> a sticking point, but as there is precedent, I'm happy with it. > > Yeah, being able to point to precedent is always helpful. > > -- > Bruce Momjian http://momjian.us > EnterpriseDB http://enterprisedb.com > > + It's impossible for everything to be true. + Attached is the patch to add a connect_timeout. I also factored out the setting of user and dbname from the default gathering loop as we only need host and port defaults and made more sense to handle user/dbname in the same area of the code as connect_timeout. This was something mentioned by Robert upthread. This also coincidentally fixes a bug in the size of the keywords and values arrays. Must have added an option during review and not extended that array. Is there a best practice to making sure that doesn't happen in the future? I was thinking define MAX_PARAMS and then setting the array size to MAX_PARAMS+1 and then checking in the getopt loop to see how many params we expect to process and erroring if we are doing to many, but that only works at runtime. One other thing I noticed while refactoring the defaults gathering code. If someone passes in a connect string for dbname, we output the wrong info at the end. This is not addressed in this patch because I wanted to get some feedback before fixing and make a separate patch. I see the only real option being to use PQconninfoParse to get the params from the connect string. If someone passes in a connect string via dbname should that have precedence over other values passed in via individual command line options? Should ordering of the command line options matter? For example if someone did: pg_isready -h foo -d "host=bar port=4321" -p 1234 What should the connection parameters be? pg_isready_timeout.diff 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] Event Triggers: adding information
On Wed, Jan 23, 2013 at 03:02:24PM -0500, Robert Haas wrote: > On Wed, Jan 23, 2013 at 2:36 PM, Bruce Momjian wrote: > > On Wed, Jan 23, 2013 at 09:33:58AM -0500, Robert Haas wrote: > >> On Wed, Jan 23, 2013 at 4:57 AM, Dimitri Fontaine > >> wrote: > >> > Thanks for commiting the fixes. About the regression tests, I think > >> > you're right, but then I can't see how to include such a test. Maybe you > >> > could add the other one, though? > >> > >> Can you point me specifically at what you have in mind so I can make > >> sure I'm considering the right thing? > >> > >> > +1 for this version, thanks. > >> > >> OK, committed that also. > > > > Also, I assume we no longer want after triggers on system tables, so I > > removed that from the TODO list and added event triggers as a completed > > item. > > Seems reasonable. Event triggers are not completed in the sense that > there is a lot more stuff we can do with this architecture, but we've > got a basic implementation now and that's progress. And they do > address the use case that triggers on system tables would have > targeted, I think, but better. Right. Users would always be chasing implementation details if they tried to trigger on system tables. -- 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] COPY FREEZE has no warning
On Wed, Jan 23, 2013 at 2:02 PM, Bruce Momjian wrote: > As a reminder, COPY FREEZE still does not issue any warning/notice if > the freezing does not happen: > > Requests copying the data with rows already frozen, just as they > would be after running the VACUUM FREEZE command. > This is intended as a performance option for initial data loading. > Rows will be frozen only if the table being loaded has been created > in the current subtransaction, there are no cursors open and there > are no older snapshots held by this transaction. If those conditions > are not met the command will continue without error though will not > freeze rows. It is also possible in rare cases that the request > cannot be honoured for internal reasons, hence FREEZE > is more of a guideline than a hard rule. > > Note that all other sessions will immediately be able to see the data > once it has been successfully loaded. This violates the normal rules > of MVCC visibility and by specifying this option the user acknowledges > explicitly that this is understood. > > Didn't we want to issue the user some kind of feedback? I believe that is what was agreed. -- 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] Event Triggers: adding information
On Wed, Jan 23, 2013 at 2:36 PM, Bruce Momjian wrote: > On Wed, Jan 23, 2013 at 09:33:58AM -0500, Robert Haas wrote: >> On Wed, Jan 23, 2013 at 4:57 AM, Dimitri Fontaine >> wrote: >> > Thanks for commiting the fixes. About the regression tests, I think >> > you're right, but then I can't see how to include such a test. Maybe you >> > could add the other one, though? >> >> Can you point me specifically at what you have in mind so I can make >> sure I'm considering the right thing? >> >> > +1 for this version, thanks. >> >> OK, committed that also. > > Also, I assume we no longer want after triggers on system tables, so I > removed that from the TODO list and added event triggers as a completed > item. Seems reasonable. Event triggers are not completed in the sense that there is a lot more stuff we can do with this architecture, but we've got a basic implementation now and that's progress. And they do address the use case that triggers on system tables would have targeted, I think, but better. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pg_isready (was: [WIP] pg_ping utility)
On Wed, Jan 23, 2013 at 02:50:01PM -0500, Phil Sorber wrote: > On Wed, Jan 23, 2013 at 1:58 PM, Bruce Momjian wrote: > > On Wed, Jan 23, 2013 at 12:27:45PM -0500, Tom Lane wrote: > >> Phil Sorber writes: > >> > On Wed, Jan 23, 2013 at 11:07 AM, Robert Haas > >> > wrote: > >> >> [rhaas pgsql]$ pg_isready -h www.google.com > >> >> > >> > >> > Do you think we should have a default timeout, or only have one if > >> > specified at the command line? > >> > >> +1 for default timeout --- if this isn't like "ping" where you are > >> expecting to run indefinitely, I can't see that it's a good idea for it > >> to sit very long by default, in any circumstance. > > > > FYI, the pg_ctl -w (wait) default is 60 seconds: > > > > from pg_ctl.c: > > > > #define DEFAULT_WAIT60 > > > > Great. That is what I came to on my own as well. Figured that might be > a sticking point, but as there is precedent, I'm happy with it. Yeah, being able to point to precedent is always helpful. -- 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] autovacuum truncate exclusive lock round two
Kevin Grittner wrote: > Applied with trivial editing, mostly from a pgindent run against > modified files. Applied back as far as 9.0. Before that code didn't match well enough for it to seem safe to apply without many hours of additional testing. I have confirmed occurences of this problem at least as far back as 9.0 in the wild, where it is causing performance degradation severe enough to force users to stop production usage long enough to manually vacuum the affected tables. The use case is a lot like what Jan described, where PostgreSQL is being used for high volume queuing. When there is a burst of activity which increases table size, and then the queues are drained back to a normal state, the problem shows up. -Kevin -- 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] pg_isready (was: [WIP] pg_ping utility)
On Wed, Jan 23, 2013 at 1:58 PM, Bruce Momjian wrote: > On Wed, Jan 23, 2013 at 12:27:45PM -0500, Tom Lane wrote: >> Phil Sorber writes: >> > On Wed, Jan 23, 2013 at 11:07 AM, Robert Haas >> > wrote: >> >> [rhaas pgsql]$ pg_isready -h www.google.com >> >> >> >> > Do you think we should have a default timeout, or only have one if >> > specified at the command line? >> >> +1 for default timeout --- if this isn't like "ping" where you are >> expecting to run indefinitely, I can't see that it's a good idea for it >> to sit very long by default, in any circumstance. > > FYI, the pg_ctl -w (wait) default is 60 seconds: > > from pg_ctl.c: > > #define DEFAULT_WAIT60 > Great. That is what I came to on my own as well. Figured that might be a sticking point, but as there is precedent, I'm happy with it. > -- > 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 -- 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: UNNEST (and other functions) WITH ORDINALITY
On Wed, Jan 23, 2013 at 02:40:45PM -0500, Bruce Momjian wrote: > On Wed, Jan 23, 2013 at 10:15:27AM -0800, David Fetter wrote: > > On Wed, Jan 23, 2013 at 03:12:37PM -0300, Alvaro Herrera wrote: > > > David Fetter wrote: > > > > On Tue, Jan 22, 2013 at 10:29:43PM -0800, David Fetter wrote: > > > > > Folks, > > > > > > > > > > Please find attached a patch which implements the SQL standard > > > > > UNNEST() WITH ORDINALITY. > > > > > > > > Added to CF4. > > > > > > Surely you meant CF 2013-Next (i.e. first commit of 9.4 cycle). > > > > I see that that's what I did, but given that this is a pretty small > > feature with low impact, I'm wondering whether it should be on CF4. > > The diff is 1.2k and has no discussion. It's been up less than a day ;) > It should be in CF 2013-Next. OK :) Cheers, David. -- David Fetter 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] Patch: UNNEST (and other functions) WITH ORDINALITY
On Wed, Jan 23, 2013 at 10:15:27AM -0800, David Fetter wrote: > On Wed, Jan 23, 2013 at 03:12:37PM -0300, Alvaro Herrera wrote: > > David Fetter wrote: > > > On Tue, Jan 22, 2013 at 10:29:43PM -0800, David Fetter wrote: > > > > Folks, > > > > > > > > Please find attached a patch which implements the SQL standard > > > > UNNEST() WITH ORDINALITY. > > > > > > Added to CF4. > > > > Surely you meant CF 2013-Next (i.e. first commit of 9.4 cycle). > > I see that that's what I did, but given that this is a pretty small > feature with low impact, I'm wondering whether it should be on CF4. The diff is 1.2k and has no discussion. It should be in CF 2013-Next. -- 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] Event Triggers: adding information
On Wed, Jan 23, 2013 at 09:33:58AM -0500, Robert Haas wrote: > On Wed, Jan 23, 2013 at 4:57 AM, Dimitri Fontaine > wrote: > > Thanks for commiting the fixes. About the regression tests, I think > > you're right, but then I can't see how to include such a test. Maybe you > > could add the other one, though? > > Can you point me specifically at what you have in mind so I can make > sure I'm considering the right thing? > > > +1 for this version, thanks. > > OK, committed that also. Also, I assume we no longer want after triggers on system tables, so I removed that from the TODO list and added event triggers as a completed item. -- 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] Visual Studio 2012 RC
On 01/23/2013 02:14 PM, Craig Ringer wrote: How have you been testing VS2012 builds? In what environment? When I tested this patch the last time I've been using Windows 8 RTM (Microsoft Windows 8 Enterprise Evaluation - 6.2.9200 Build 9200) and Microsoft Visual Studio Express 2012 für Windows Desktop RTM (Version 11.0.50727.42) Regards, Brar
Re: [HACKERS] bugfix: --echo-hidden is not supported by \sf statements
2013/1/14 Tom Lane : > Robert Haas writes: >> On Mon, Jan 14, 2013 at 11:56 AM, Tom Lane wrote: >>> So far as I can tell, get_create_function_cmd (and lookup_function_oid >>> too) were intentionally designed to not show their queries, and for that >>> matter they go out of their way to produce terse error output if they >>> fail. I'm not sure why we should revisit that choice. In any case >>> it seems silly to change one and not the other. > >> Agreed on the second point, but I think I worked on that patch, and I >> don't think that was intentional on my part. You worked on it, too, >> IIRC, so I guess you'll have to comment on your intentions. > >> Personally I think -E is one of psql's finer features, so +1 from me >> for making it apply across the board. > > Well, fine, but then it should fix both of them and remove > minimal_error_message altogether. I would however suggest eyeballing > what happens when you try "\ef nosuchfunction" (with or without -E). > I'm pretty sure that the reason for the special error handling in these > functions is that the default error reporting was unpleasant for this > use-case. so I rewrote patch still is simple On the end I didn't use PSQLexec - just write hidden queries directly from related functions - it is less invasive Regards Pavel > > regards, tom lane psql_sf_hidden_queries.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_ctl idempotent option
On Wed, Jan 23, 2013 at 09:00:25PM +0200, Heikki Linnakangas wrote: > On 23.01.2013 20:56, Bruce Momjian wrote: > >On Tue, Jan 22, 2013 at 06:03:28PM +0530, Ashutosh Bapat wrote: > >>anyway, +1 for making this as default option. Going that path, would > >>we be breaking backward compatibility? There might be scripts, (being > >>already used), which depend upon the current behaviour. > > > >FYI, I have a pg_upgrade patch that relies on the old throw-an-error > >behavior. Will there be a way to still throw an error if we make > >idempotent the default? > > Could you check the status with "pg_ctl status" first, and throw an > error if it's not what you expected? Well, this could still create a period of time where someone else starts the server between my status and my starting it. Do we really want that? And what if I want to start it with my special -o parameters, and I then can't tell if it was already running or it is using my parameters. I think an idempotent default is going to cause problems. -- 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
[HACKERS] COPY FREEZE has no warning
As a reminder, COPY FREEZE still does not issue any warning/notice if the freezing does not happen: Requests copying the data with rows already frozen, just as they would be after running the VACUUM FREEZE command. This is intended as a performance option for initial data loading. Rows will be frozen only if the table being loaded has been created in the current subtransaction, there are no cursors open and there are no older snapshots held by this transaction. If those conditions are not met the command will continue without error though will not freeze rows. It is also possible in rare cases that the request cannot be honoured for internal reasons, hence FREEZE is more of a guideline than a hard rule. Note that all other sessions will immediately be able to see the data once it has been successfully loaded. This violates the normal rules of MVCC visibility and by specifying this option the user acknowledges explicitly that this is understood. Didn't we want to issue the user some kind of feedback? -- 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] pg_ctl idempotent option
On 23.01.2013 20:56, Bruce Momjian wrote: On Tue, Jan 22, 2013 at 06:03:28PM +0530, Ashutosh Bapat wrote: anyway, +1 for making this as default option. Going that path, would we be breaking backward compatibility? There might be scripts, (being already used), which depend upon the current behaviour. FYI, I have a pg_upgrade patch that relies on the old throw-an-error behavior. Will there be a way to still throw an error if we make idempotent the default? Could you check the status with "pg_ctl status" first, and throw an error if it's not what you expected? - 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] [PATCH] pg_isready (was: [WIP] pg_ping utility)
On Wed, Jan 23, 2013 at 12:27:45PM -0500, Tom Lane wrote: > Phil Sorber writes: > > On Wed, Jan 23, 2013 at 11:07 AM, Robert Haas wrote: > >> [rhaas pgsql]$ pg_isready -h www.google.com > >> > > > Do you think we should have a default timeout, or only have one if > > specified at the command line? > > +1 for default timeout --- if this isn't like "ping" where you are > expecting to run indefinitely, I can't see that it's a good idea for it > to sit very long by default, in any circumstance. FYI, the pg_ctl -w (wait) default is 60 seconds: from pg_ctl.c: #define DEFAULT_WAIT60 -- 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] pg_ctl idempotent option
On Tue, Jan 22, 2013 at 06:03:28PM +0530, Ashutosh Bapat wrote:
> On Tue, Jan 15, 2013 at 9:36 PM, Tom Lane wrote:
> > Peter Eisentraut writes:
> >> On 1/14/13 10:22 AM, Tom Lane wrote:
> >>> Also it appears to me that the hunk at lines 812ff is changing the
> >>> default behavior, which is not what the patch is advertised to do.
> >
> >> True, I had forgotten to mention that.
> >
> >> Since it's already the behavior for start, another option would be to
> >> just make it the default for stop as well and forget about the extra
> >> options.
>
> By default, (without idempotent option), if it finds the pid, it tries
> to start one. If there is already one running, it exits with errorcode
> 1, otherwise it has already run the server.
> 814 exitcode = start_postmaster();
> 815 if (exitcode != 0)
> 816 {
> 817 write_stderr(_("%s: could not start server: exit code was %d\n"),
> 818 progname, exitcode);
> 819 exit(1);
> 820 }
>
> What we can do under idempotent is to return with code 0 instead of
> exit(1) above, thus not need the changes in the patch around line 812.
> That will be more in-line with the description at
> http://www.postgresql.org/message-id/1253165415.18853.32.ca...@vanquo.pezone.net
>
> > for example an exit
> > code of 0 for an attempt to start a server which is already running
> > or an attempt to stop a server which isn't running. (These are only
> > two examples of differences between what is required of an LSB
> > conforming init script and what pg_ctl does. Are we all in universal
> > agreement to make such a change to pg_ctl?
>
> anyway, +1 for making this as default option. Going that path, would
> we be breaking backward compatibility? There might be scripts, (being
> already used), which depend upon the current behaviour.
FYI, I have a pg_upgrade patch that relies on the old throw-an-error
behavior. Will there be a way to still throw an error if we make
idempotent the default?
--
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] Support for REINDEX CONCURRENTLY
On 24/01/13 07:45, Alvaro Herrera wrote: Andres Freund escribió: I somewhat dislike the fact that CONCURRENTLY isn't really concurrent here (for the listeners: swapping the indexes acquires exlusive locks) , but I don't see any other naming being better. REINDEX ALMOST CONCURRENTLY? REINDEX BEST EFFORT CONCURRENTLY? -- 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] Support for REINDEX CONCURRENTLY
Andres Freund escribió: > I somewhat dislike the fact that CONCURRENTLY isn't really concurrent > here (for the listeners: swapping the indexes acquires exlusive locks) , > but I don't see any other naming being better. REINDEX ALMOST CONCURRENTLY? -- Á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] CF3+4 (was Re: Parallel query execution)
* Robert Haas (robertmh...@gmail.com) wrote: > For all of that, I'm not sure that people failing to seek consensus > before coding is really so much of a problem as you seem to think. For my part, I don't think the lack of consensus-finding before submitting patches is, in itself, a problem. The problem, imv, is that everyone is expecting that once they've written a patch and put it on a commitfest that it's going to get committed- and it seems like committers are feeling under pressure that, because something's on the CF app, it needs to be committed in some form. There's a lot of good stuff out there, sure, and even more good *ideas*, but it's important to make sure we can provide a stable system with regular releases. As discussed, we really need to be ready to truely triage the remaining patch set, figure out who is going to work on what, and punt the rest til post-9.3. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Support for REINDEX CONCURRENTLY
On 2013-01-15 18:16:59 +0900, Michael Paquier wrote:
> OK. I am back to this patch after a too long time.
Dito ;)
> > > > * would be nice (but thats probably a step #2 thing) to do the
> > > > individual steps of concurrent reindex over multiple relations to
> > > > avoid too much overall waiting for other transactions.
> > > >
> > > I think I did that by now using one transaction per index for each
> > > operation except the drop phase...
> >
> > Without yet having read the new version, I think thats not what I
> > meant. There currently is a wait for concurrent transactions to end
> > after most of the phases for every relation, right? If you have a busy
> > database with somewhat longrunning transactions thats going to slow
> > everything down with waiting quite bit. I wondered whether it would make
> > sense to do PHASE1 for all indexes in all relations, then wait once,
> > then PHASE2...
>
> That obviously has some space and index maintainece overhead issues, but
> > its probably sensible anyway in many cases.
> >
> OK, phase 1 is done with only one transaction for all the indexes. Do you
> mean that we should do that with a single transaction for each index?
Yes.
> > Isn't the following block content thats mostly available somewhere else
> > already?
> > [... doc extract ...]
> >
> Yes, this portion of the docs is pretty similar to what is findable in
> CREATE INDEX CONCURRENTLY. Why not creating a new common documentation
> section that CREATE INDEX CONCURRENTLY and REINDEX CONCURRENTLY could refer
> to? I think we should first work on the code and then do the docs properly
> though.
Agreed. I just noticed it when scrolling through the patch.
> > > - if (concurrent && is_exclusion)
> > > + if (concurrent && is_exclusion && !is_reindex)
> > > ereport(ERROR,
> > > (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> > >errmsg_internal("concurrent index
> > creation for exclusion constraints is not supported")));
> >
> > This is what I referred to above wrt reindex and CONCURRENTLY. We
> > shouldn't pass concurrently if we don't deem it to be safe for exlusion
> > constraints.
> >
> So does that mean that it is not possible to create an exclusive constraint
> in a concurrent context?
Yes, its currently not safe in the general case.
> Code path used by REINDEX concurrently permits to
> create an index in parallel of an existing one and not a completely new
> index. Shouldn't this work for indexes used by exclusion indexes also?
But that fact might safe things. I don't immediately see any reason that
adding a
if (!indisvalid)
return;
to check_exclusion_constraint wouldn't be sufficient if there's another
index with an equivalent definition.
> > > + /*
> > > + * Phase 2 of REINDEX CONCURRENTLY
> > > + *
> > > + * Build concurrent indexes in a separate transaction for each
> > index to
> > > + * avoid having open transactions for an unnecessary long time.
> > We also
> > > + * need to wait until no running transactions could have the
> > parent table
> > > + * of index open. A concurrent build is done for each concurrent
> > > + * index that will replace the old indexes.
> > > + */
> > > +
> > > + /* Get the first element of concurrent index list */
> > > + lc2 = list_head(concurrentIndexIds);
> > > +
> > > + foreach(lc, indexIds)
> > > + {
> > > + RelationindexRel;
> > > + Oid indOid = lfirst_oid(lc);
> > > + Oid concurrentOid = lfirst_oid(lc2);
> > > + Oid relOid;
> > > + boolprimary;
> > > + LOCKTAG*heapLockTag = NULL;
> > > + ListCell *cell;
> > > +
> > > + /* Move to next concurrent item */
> > > + lc2 = lnext(lc2);
> > > +
> > > + /* Start new transaction for this index concurrent build */
> > > + StartTransactionCommand();
> > > +
> > > + /* Get the parent relation Oid */
> > > + relOid = IndexGetRelation(indOid, false);
> > > +
> > > + /*
> > > + * Find the locktag of parent table for this index, we
> > need to wait for
> > > + * locks on it.
> > > + */
> > > + foreach(cell, lockTags)
> > > + {
> > > + LOCKTAG *localTag = (LOCKTAG *) lfirst(cell);
> > > + if (relOid == localTag->locktag_field2)
> > > + heapLockTag = localTag;
> > > + }
> > > +
> > > + Assert(heapLockTag && heapLockTag->locktag_field2 !=
> > InvalidOid);
> > > + WaitForVirtualLocks(*heapLockTag, ShareLock);
> >
> > Why do we have to do the WaitForVirtualLocks here? Shouldn't we do this
> > once for all relations after each phase? Otherwise the waiting time will
> > real
Re: [HACKERS] foreign key locks
I just pushed this patch to the master branch. There was a corresponding catversion bump and pg_control version bump. I have verified that "make check-world" passes on my machine, as well as isolation tests and pg_upgrade. Tom Lane said at one point "this is too complex to maintain". Several times during the development I feared he might well be right. I am sure he will be discovering many oversights and poor design choices, when gets around to reviewing the code; hopefully some extra effort will be all that's needed to make the whole thing work sanely and not eat anyone's data. I just hope that nothing so serious comes up that the patch will need to be reverted completely. This patch is the result of the work of many people. I am not allowed to mention the patch sponsors in the commit message, so I'm doing it here: first and foremost I need to thank my previous and current employers Command Prompt and 2ndQuadrant -- they were extremely kind in allowing me to work on this for days on end (and not all of it was supported by financial sponsors). Joel Jacobson started the whole effort by posting a screencast of a problem his company was having; I hope they found a workaround in the meantime, because his post was in mid 2010. The key idea of this patch' design came from Simon Riggs; Noah Misch provided additional design advice that allowed the project torun to completion. Noah and Andres Freund spent considerable time reviewing early versions of this patch; they uncovered many embarrasing bugs in my implementation. Kevin Grittner, Robert Haas, and others, provided useful comments as well. Noah Misch, Andres Freund, Marti Raudsepp and Alexander Shulgin also provided bits of code. Any bugs that remain are, of course, my own fault only. Financial support came from * Command Prompt Inc. of Washington, US * 2ndQuadrant Ltd. of United Kingdom * Trustly (then Glue Finance) of Sweden * Novozymes A/S of Denmark * MailerMailer LLC of Maryland, US * PostgreSQL Experts, Inc. of California, US. My sincerest thanks to everyone. I seriously hope that no patch of mine ever becomes this monstruous again. -- Á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] CF3+4 (was Re: Parallel query execution)
On 01/23/2013 09:51 AM, Josh Berkus wrote: The only way to fix increasing bug counts is through more-comprehensive regular testing. Currently we have regression/unit tests which cover maybe 30% of our code. Performance testing is largely ad-hoc. We don't require comprehensive acceptance testing for new patches. And we have > 1m lines of code. Of course our bug count is increasing. And... slow down the release cycle or slow down the number of features that are accepted. Don't get me wrong I love everything we have and are adding every cycle but there does seem to be a definite weight difference between # of features added and time spent allowing those features to settle. Sincerely, JD -- Command Prompt, Inc. - http://www.commandprompt.com/ PostgreSQL Support, Training, Professional Services and Development High Availability, Oracle Conversion, Postgres-XC @cmdpromptinc - 509-416-6579 -- 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] Re: Proposal for Allow postgresql.conf values to be changed via SQL [review]
Fujii Masao escribió: > On Wed, Jan 23, 2013 at 12:24 PM, Amit Kapila wrote: > >> Is it safe to write something in the directory other than data > >> directory > >> via SQL? > >> > >> postgres user usually has the write permission for the configuration > >> directory like /etc/postgresql? > > > > As postgresql.conf will also be in same path and if user can change that, > > then what's the problem with postgresql.auto.conf > > If the configuration directory like /etc is owned by root and only root has > a write permission for it, the user running PostgreSQL server would not > be able to update postgresql.auto.conf there. OTOH, even in that case, > a user can switch to root and update the configuration file there. I'm not > sure whether the configuration directory is usually writable by the user > running PostgreSQL server in Debian or Ubuntu, though. Yes it is -- the /etc/postgresql// directory (where postgresql.conf resides) is owned by user postgres. -- Á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] BUG #6510: A simple prompt is displayed using wrong charset
On 01/23/2013 01:08 PM, Alvaro Herrera wrote: Tom Lane escribió: Alexander Law writes: Please let me know if I can do something to get the bug fix (https://commitfest.postgresql.org/action/patch_view?id=902) committed. It's waiting on some Windows-savvy committer to pick it up, IMO. (FWIW, I have no objection to the patch as given, but I am unqualified to evaluate how sane it is on Windows.) I think part of the problem is that we no longer have many Windows-savvy committers willing to spend time on Windows-specific patches; there are, of course, people with enough knowledge, but they don't always necessarily have much interest. Note that I added Magnus and Andrew to this thread because they are known to have contributed Windows-specific improvements, but they have yet to followup on this thread *at all*. I remember back when Windows support was added, one of the arguments in its favor was "it's going to attract lots of new developers". Yeah, right. I'm about to take a look at this one. Note BTW that Craig Ringer has recently done some excellent work on Windows, and there are several other active non-committers (e.g. Noah) who comment on Windows too. IIRC I never expected us to get a huge influx of developers from the Windows world. What I did expect was a lot of new users, and I think we have on fact got that. 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] pg_basebackup with -R option and start standby have problems with escaped password
On Wed, Jan 23, 2013 at 10:18 AM, Hari Babu wrote: > Test scenario to reproduce: > 1. Start the server > 2. create the user as follows > ./psql postgres -c "create user user1 superuser login > password 'use''1'" > > 3. Take the backup with -R option as follows. > ./pg_basebackup -D ../../data1 -R -U user1 -W > > The following errors are occurring when the new standby on the backup > database starts. > > FATAL: could not connect to the primary server: missing "=" after "1'" in > connection info string What does the resulting recovery.conf file look like? -- 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] Patch: UNNEST (and other functions) WITH ORDINALITY
On Wed, Jan 23, 2013 at 03:12:37PM -0300, Alvaro Herrera wrote: > David Fetter wrote: > > On Tue, Jan 22, 2013 at 10:29:43PM -0800, David Fetter wrote: > > > Folks, > > > > > > Please find attached a patch which implements the SQL standard > > > UNNEST() WITH ORDINALITY. > > > > Added to CF4. > > Surely you meant CF 2013-Next (i.e. first commit of 9.4 cycle). I see that that's what I did, but given that this is a pretty small feature with low impact, I'm wondering whether it should be on CF4. Cheers, David. -- David Fetter 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] Patch: UNNEST (and other functions) WITH ORDINALITY
David Fetter wrote: > On Tue, Jan 22, 2013 at 10:29:43PM -0800, David Fetter wrote: > > Folks, > > > > Please find attached a patch which implements the SQL standard > > UNNEST() WITH ORDINALITY. > > Added to CF4. Surely you meant CF 2013-Next (i.e. first commit of 9.4 cycle). -- Á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] Patch: clean up addRangeTableEntryForFunction
On Tue, Jan 22, 2013 at 11:02:18PM -0500, Tom Lane wrote: > David Fetter writes: > > I've been working with Andrew Gierth (well, mostly he's been doing > > the work, as usual) to add WITH ORDINALITY as an option for > > set-returning functions. In the process, he found a minor > > opportunity to clean up the interface for $SUBJECT, reducing the > > call to a Single Point of Truth for lateral-ness, very likely > > improving the efficiency of calls to that function. > > As I mentioned in our off-list discussion, I think this is going in > the wrong direction. It'd make more sense to me to get rid of the > RangeFunction parameter, instead passing the two fields that > addRangeTableEntryForFunction actually uses out of that. With utmost respect, of the four fields currently in RangeFunction: type (tag), lateral, funccallnode, alias, and coldeflist, the function needs three (all but funccallnode, which has already been transformed into a funcexpr). The patch for ordinality makes that 4/5 with the ordinality field added. > If we do what you have here, we'll be welding together the alias and > lateral settings for the new RTE; which conceivably some other > caller would want to specify in a different way. As a comparison > point, you might want to look at the various calls to > addRangeTableEntryForSubquery: some of those pass multiple fields > out of the same RangeSubselect, and some do not. As to addRangeTableEntryForSubquery, I'm not seeing the connection to the case at hand. Could you please spell it out? Cheers, David. -- David Fetter 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] BUG #6510: A simple prompt is displayed using wrong charset
Tom Lane escribió: > Alexander Law writes: > > Please let me know if I can do something to get the bug fix > > (https://commitfest.postgresql.org/action/patch_view?id=902) committed. > > It's waiting on some Windows-savvy committer to pick it up, IMO. > (FWIW, I have no objection to the patch as given, but I am unqualified > to evaluate how sane it is on Windows.) I think part of the problem is that we no longer have many Windows-savvy committers willing to spend time on Windows-specific patches; there are, of course, people with enough knowledge, but they don't always necessarily have much interest. Note that I added Magnus and Andrew to this thread because they are known to have contributed Windows-specific improvements, but they have yet to followup on this thread *at all*. I remember back when Windows support was added, one of the arguments in its favor was "it's going to attract lots of new developers". Yeah, right. -- Á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] [COMMITTERS] pgsql: Improve concurrency of foreign key locking
Andrew Dunstan wrote:
>
> On 01/23/2013 10:12 AM, Alvaro Herrera wrote:
> >Improve concurrency of foreign key locking
>
> This error message change looks rather odd, and has my head spinning a bit:
>
> -errmsg("SELECT FOR UPDATE/SHARE cannot be
> applied to the nullable side of an outer join")));
> +errmsg("SELECT FOR UPDATE/SHARE/KEY UPDATE/KEY
> SHARE cannot be applied to the nullable side of an outer join")))
>
> Can't we do better than that?
Basically this message says "a SELECT with a locking clause attached
cannot be blah blah". There are many messages that had the original
code saying "SELECT FOR UPDATE cannot be blah blah", which was later
(8.1) updated to say "SELECT FOR UPDATE/SHARE cannot be blah blah". I
expanded them using the same logic, but maybe there's a better
suggestion? Note that the SELECT reference page now has a subsection
called "The locking clause", so maybe it's okay to use that term now.
--
Á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] proposal: fix corner use case of variadic fuctions usage
2013/1/23 Tom Lane :
> Pavel Stehule writes:
>> next related example
>
>> CREATE OR REPLACE FUNCTION public.myleast(VARIADIC integer[])
>> RETURNS integer
>> LANGUAGE sql
>> AS $function$
>> select min(v) from unnest($1) g(v)
>> $function$
>
> The reason you get a null from that is that (1) unnest() produces zero
> rows out for either a null or empty-array input, and (2) min() over
> zero rows produces NULL.
>
> In a lot of cases, it's not very sane for aggregates over no rows to
> produce NULL; the best-known example is that SUM() produces NULL, when
> anyone who'd not suffered brain-damage from sitting on the SQL committee
> would have made it return zero. So I'm not very comfortable with
> generalizing from this specific case to decide that NULL is the
> universally right result.
>
I am testing some situation and there are no consistent idea - can we
talk just only about functions concat and concat_ws?
these functions are really specific - now we talk about corner use
case and strongly PostgreSQL proprietary solution. So any solution
should not too bad.
Difference between all solution will by maximally +/- 4 simple rows
per any function.
Possibilities
1) A. concat(variadic NULL) => empty string, B. concat(variadic '{}')
=> empty string -- if we accept @A, then B is ok
2) A. concat(variadic NULL) => NULL, B. concat(variadic '{}') => NULL
-- question - is @B valid ?
3) A. concat(variadic NULL) => NULL, B. concat(variadic '{}) => empty string
There are no other possibility.
I can live with any variant - probably we find any precedent to any
variant in our code or in ANSI SQL.
I like @2 as general concept for PostgreSQL variadic functions, but
when we talk about concat() and concat_ws() @1 is valid too.
Please, can somebody say his opinion early
Regards
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
Re: [HACKERS] CF3+4 (was Re: Parallel query execution)
On 01/23/2013 09:08 AM, Andres Freund wrote: > On 2013-01-23 11:44:29 -0500, Robert Haas wrote: >> On Tue, Jan 22, 2013 at 1:15 AM, Tom Lane wrote: >>> Yeah, and a lot more fairly-new developers who don't understand all the >>> connections in the existing system. Let me just push back a bit here: >>> based on the amount of time I've had to spend fixing bugs over the past >>> five months, 9.2 was our worst release ever. I don't like that trend, >>> and I don't want to see it continued because we get laxer about >>> accepting patches. IMO we are probably too lax already. >> >> Really? Hmm, that's not good. I seem to recall 8.4.x being pretty >> bad, and some of the recent bugs we fixed were actually 9.1.x problems >> that slipped through the cracks. > > FWIW I concur with Tom's assessment. The only way to fix increasing bug counts is through more-comprehensive regular testing. Currently we have regression/unit tests which cover maybe 30% of our code. Performance testing is largely ad-hoc. We don't require comprehensive acceptance testing for new patches. And we have > 1m lines of code. Of course our bug count is increasing. I'm gonna see if I can do something about improving our test coverage. -- 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] My first patch! (to \df output)
On Wed, Jan 23, 2013 at 12:31 AM, Jon Erdman wrote: > -BEGIN PGP SIGNED MESSAGE- > Hash: SHA1 > > > Done. Attached. > - -- > Jon T Erdman (aka StuckMojo) > PostgreSQL Zealot > > On 01/22/2013 11:17 PM, Phil Sorber wrote: >> On Wed, Jan 23, 2013 at 12:10 AM, Jon Erdman >> wrote: >> >> Updated the patch in commitfest with the doc change, and added a >> comment to explain the whitespace change (it was to clean up the >> sql indentation). I've also attached the new patch here for >> reference. >> >>> Docs looks good. Spaces gone. >> >>> Still need to replace 'definer' and 'invoker' with %s and add >>> the corresponding gettext_noop() calls I think. >> This looks good to me now. Compiles and works as described. One thing I would note for the future though, when updating a patch, add a version to the file name to distinguish it from older versions of the patch. > -BEGIN PGP SIGNATURE- > Comment: Using GnuPG with undefined - http://www.enigmail.net/ > > iEYEARECAAYFAlD/dcoACgkQRAk1+p0GhSEKHQCZAW8UNqSjYxBgBvt2nuffrkrV > +9AAn1hChpY5Jg8G8T3XmlIb+3VUSEQ2 > =3cFD > -END PGP SIGNATURE- -- 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] [COMMITTERS] pgsql: Improve concurrency of foreign key locking
On 23 January 2013 17:15, Andres Freund wrote:
> On 2013-01-23 11:58:28 -0500, Andrew Dunstan wrote:
>>
>> On 01/23/2013 10:12 AM, Alvaro Herrera wrote:
>> >Improve concurrency of foreign key locking
>>
>> This error message change looks rather odd, and has my head spinning a bit:
>>
>> -errmsg("SELECT FOR UPDATE/SHARE cannot be applied to
>> the nullable side of an outer join")));
>> +errmsg("SELECT FOR UPDATE/SHARE/KEY UPDATE/KEY SHARE
>> cannot be applied to the nullable side of an outer join")))
>>
>> Can't we do better than that?
>
> I don't really see how? I don't think listing only the current locklevel
> really is an improvement and something like "SELECT ... FOR $locktype
> cannot .." seem uncommon enough in pg error messages to be strange.
> Now I aggree that listing all those locklevels isn't that nice, but I
> don't really have a better idea.
"row level locks cannot be applied to the NULLable side of an outer join"
Hint: there are no rows to lock
--
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
Re: [HACKERS] [COMMITTERS] pgsql: Improve concurrency of foreign key locking
Andres Freund writes:
> On 2013-01-23 11:58:28 -0500, Andrew Dunstan wrote:
>> This error message change looks rather odd, and has my head spinning a bit:
>>
>> -errmsg("SELECT FOR UPDATE/SHARE cannot be applied to
>> the nullable side of an outer join")));
>> +errmsg("SELECT FOR UPDATE/SHARE/KEY UPDATE/KEY SHARE
>> cannot be applied to the nullable side of an outer join")))
>>
>> Can't we do better than that?
> I don't really see how? I don't think listing only the current locklevel
> really is an improvement and something like "SELECT ... FOR $locktype
> cannot .." seem uncommon enough in pg error messages to be strange.
> Now I aggree that listing all those locklevels isn't that nice, but I
> don't really have a better idea.
I don't really see what's wrong with the original spelling of the
message. The fact that you can now insert KEY in there doesn't really
affect anything for the purposes of this error.
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] [PATCH] Add Makefile dep in bin/scripts for libpgport
I get the following error when I try to compile just a specific binary in src/bin/scripts: gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard reindexdb.o common.o dumputils.o kwlookup.o keywords.o -L../../../src/port -lpgport -L../../../src/interfaces/libpq -lpq -L../../../src/port -Wl,--as-needed -Wl,-rpath,'/usr/local/pgsql/lib',--enable-new-dtags -lpgport -lz -lreadline -lcrypt -ldl -lm -o reindexdb /usr/bin/ld: cannot find -lpgport /usr/bin/ld: cannot find -lpgport collect2: error: ld returned 1 exit status make: *** [reindexdb] Error 1 It appears it is missing the libpgport dependency. Attached is a patch to correct that. This is not normally a problem because when building the whole tree libpgport is usually compiled already. scripts_makefile_pgport_dep.diff 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] BUG #6510: A simple prompt is displayed using wrong charset
Alexander Law writes: > Please let me know if I can do something to get the bug fix > (https://commitfest.postgresql.org/action/patch_view?id=902) committed. It's waiting on some Windows-savvy committer to pick it up, IMO. (FWIW, I have no objection to the patch as given, but I am unqualified to evaluate how sane it is on Windows.) 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] pg_isready (was: [WIP] pg_ping utility)
Phil Sorber writes: > On Wed, Jan 23, 2013 at 11:07 AM, Robert Haas wrote: >> [rhaas pgsql]$ pg_isready -h www.google.com >> > Do you think we should have a default timeout, or only have one if > specified at the command line? +1 for default timeout --- if this isn't like "ping" where you are expecting to run indefinitely, I can't see that it's a good idea for it to sit very long by default, in any circumstance. 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] CF3+4 (was Re: Parallel query execution)
On Mon, Jan 21, 2013 at 02:04:14PM -0500, Tom Lane wrote: > Josh Berkus writes: > >> IMHO that's the single most important task of a review. > > > Really? I'd say the most important task for a review is "does the patch > > do what it says it does?". That is, if the patch is supposed to > > implement feature X, does it actually? If it's a performance patch, > > does performance actually improve? > > > If the patch doesn't implement what it's supposed to, who cares what the > > code looks like? > > But even before that, you have to ask whether what it's supposed to do > is something we want. Yep. Our TODO list has a pretty short summary of this at the top: Desirability -> Design -> Implement -> Test -> Review -> Commit -- 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] [PATCH] pg_isready (was: [WIP] pg_ping utility)
On Wed, Jan 23, 2013 at 11:07 AM, Robert Haas wrote: > On Mon, Jan 21, 2013 at 12:23 PM, Phil Sorber wrote: >> Changing up the subject line because this is no longer a work in >> progress nor is it pg_ping anymore. > > OK, I committed this. However, I have one suggestion. Maybe it would > be a good idea to add a -c or -t option that sets the connect_timeout > parameter. Because: > > [rhaas pgsql]$ pg_isready -h www.google.com > Oh, hrmm. Yes, I will address that with a follow up patch. I guess in my testing I was using a host that responded properly with port unreachable or TCP RST or something. Do you think we should have a default timeout, or only have one if specified at the command line? > > -- > 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] [COMMITTERS] pgsql: Improve concurrency of foreign key locking
On 2013-01-23 11:58:28 -0500, Andrew Dunstan wrote:
>
> On 01/23/2013 10:12 AM, Alvaro Herrera wrote:
> >Improve concurrency of foreign key locking
>
> This error message change looks rather odd, and has my head spinning a bit:
>
> -errmsg("SELECT FOR UPDATE/SHARE cannot be applied to
> the nullable side of an outer join")));
> +errmsg("SELECT FOR UPDATE/SHARE/KEY UPDATE/KEY SHARE
> cannot be applied to the nullable side of an outer join")))
>
> Can't we do better than that?
I don't really see how? I don't think listing only the current locklevel
really is an improvement and something like "SELECT ... FOR $locktype
cannot .." seem uncommon enough in pg error messages to be strange.
Now I aggree that listing all those locklevels isn't that nice, but I
don't really have a better idea.
Andres
--
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] CF3+4 (was Re: Parallel query execution)
On 2013-01-23 11:44:29 -0500, Robert Haas wrote: > On Tue, Jan 22, 2013 at 1:15 AM, Tom Lane wrote: > > Yeah, and a lot more fairly-new developers who don't understand all the > > connections in the existing system. Let me just push back a bit here: > > based on the amount of time I've had to spend fixing bugs over the past > > five months, 9.2 was our worst release ever. I don't like that trend, > > and I don't want to see it continued because we get laxer about > > accepting patches. IMO we are probably too lax already. > > Really? Hmm, that's not good. I seem to recall 8.4.x being pretty > bad, and some of the recent bugs we fixed were actually 9.1.x problems > that slipped through the cracks. FWIW I concur with Tom's assessment. > On Tue, Jan 22, 2013 at 1:15 AM, Tom Lane wrote: > > For a very long time we've tried to encourage people to submit rough > > ideas to pgsql-hackers for discussion *before* they start coding. > > The point of that is to weed out, or fix if possible, (some of) the bad > > ideas before a lot of effort has gotten expended on them. It seems > > though that less and less of that is actually happening, so I wonder > > whether the commitfest process is encouraging inefficiency by making > > people think they should start a discussion with a mostly-complete > > patch. Or maybe CF review is just crowding out any serious discussion > > of rough ideas. There was some discussion at the last dev meeting of > > creating a "design review" process within commitfests, but nothing got > > done about it ... Are you thinking of specific patches? From what I remember all all the bigger patches arround the 9.3 cycle were quite heavily discussed during multiple stages of their development. I agree that its not necessarily the case for smaller patches but at least for me in those cases its often hard to judge how complex something is before doing an initial prototype. One aspect of this might be that its sometimes easier to convince -hackers of some idea if you can prove its doable. Another that the amount of bikeshedding seems to be far lower if a patch already shapes a feature in some way. > I think there's been something of a professionalization of PostgreSQL > development over the last few years. More and more people are able > to get paid to work on PostgreSQL as part or in a few cases all of > their job. This trend includes me, but also a lot of other people. Yes. > There are certainly good things about this, but I think it increases > the pressure to get patches committed. If you are developing a patch > on your own time and it doesn't go in, it may be annoying but that's > about it. If you're getting paid to get that patch committed and it > doesn't go in, either your boss cares or your customer cares, or both, > so now you have a bigger problem. And it also might shape the likelihood of getting paid for future work, be that a specific patch or time for hacking/maintenance. > For all of that, I'm not sure that people failing to seek consensus > before coding is really so much of a problem as you seem to think. A > lot of the major efforts underway for this release have been discussed > extensively on multiple threads. The fact that some of those ideas > may be less than half-baked at this point may in some cases be the > submitter's fault, but there also cases where it's just that they > haven't got enough looking-at from the people who know enough to > evaluate them in detail, and perhaps some cases that are really > nobody's fault: nothing in life is going to be perfect all the time no > matter how hard everyone tries. I agree. Take logical replication/decoding for example. While we developed a prototype first, we/I tried to take in as much feedback from the community as we could. Just about no code, and only few concepts, from the initial prototype remain, and thats absolutely good. I don't think a more "talk about it first" approach would have helped here. Do others disagree? But as soon as the rough, rough design was laid out the amount of specific feedback shrank. Part of that is that some people involved in the discussions had changes in their work situation that influenced the amount of time they could spend on it, but I think one other problem is that at some point it gets hard to give feedback on a complex, evolving patch without it eating up big amount of time. 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] BUG #6510: A simple prompt is displayed using wrong charset
Hello, Please let me know if I can do something to get the bug fix (https://commitfest.postgresql.org/action/patch_view?id=902) committed. I would like to fix other bugs related to postgres localization, but I am not sure yet how to do it. Thanks in advance, Alexander 18.10.2012 19:46, Alvaro Herrera wrote: Noah Misch escribió: Following an off-list ack from Alexander, here is that version. No functional differences from Alexander's latest version, and I have verified that it still fixes the original test case. I'm marking this Ready for Committer. This seems good to me, but I'm not comfortable committing Windows stuff. Andrew, Magnus, are you able to handle this? -- 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] [COMMITTERS] pgsql: Improve concurrency of foreign key locking
On 01/23/2013 10:12 AM, Alvaro Herrera wrote:
Improve concurrency of foreign key locking
This error message change looks rather odd, and has my head spinning a bit:
-errmsg("SELECT FOR UPDATE/SHARE cannot be applied
to the nullable side of an outer join")));
+errmsg("SELECT FOR UPDATE/SHARE/KEY UPDATE/KEY
SHARE cannot be applied to the nullable side of an outer join")))
Can't we do better than that?
(It's also broken my FDW check, but I'll fix that when this is sorted out)
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: fix corner use case of variadic fuctions usage
Pavel Stehule writes: > next related example > CREATE OR REPLACE FUNCTION public.myleast(VARIADIC integer[]) > RETURNS integer > LANGUAGE sql > AS $function$ > select min(v) from unnest($1) g(v) > $function$ The reason you get a null from that is that (1) unnest() produces zero rows out for either a null or empty-array input, and (2) min() over zero rows produces NULL. In a lot of cases, it's not very sane for aggregates over no rows to produce NULL; the best-known example is that SUM() produces NULL, when anyone who'd not suffered brain-damage from sitting on the SQL committee would have made it return zero. So I'm not very comfortable with generalizing from this specific case to decide that NULL is the universally right result. 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] Prepared statements fail after schema changes with surprising error
On Wed, Jan 23, 2013 at 11:40 AM, Tom Lane wrote: > Yeah, that is probably the major hazard IMO too. The designs sketched > in this thread would be sufficient to ensure that DDL in one session's > temp schema wouldn't have to invalidate plans in other sessions --- but > is that good enough? > > Your point that the locking code doesn't quite cope with newly-masked > objects makes me feel that we could get away with not solving the case > for plan caching either. Or at least that we could put off the problem > till another day. If we are willing to just change plancache's handling > of search_path, that's a small patch that I think is easily doable for > 9.3. If we insist on installing schema-level invalidation logic, it's > not happening before 9.4. I agree with that analysis. FWIW, I am pretty confident that the narrower fix will make quite a few people significantly happier than they are today, so if you're willing to take that on, +1 from me. I believe the search-path-interpolation problem is a sufficiently uncommon case that, in practice, it rarely comes up. That's not to say that we shouldn't ever fix it, but I think the simpler fix will be a 90% solution and people will be happy to have made that much progress this cycle. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] CF3+4 (was Re: Parallel query execution)
On Tue, Jan 22, 2013 at 1:15 AM, Tom Lane wrote: > Yeah, and a lot more fairly-new developers who don't understand all the > connections in the existing system. Let me just push back a bit here: > based on the amount of time I've had to spend fixing bugs over the past > five months, 9.2 was our worst release ever. I don't like that trend, > and I don't want to see it continued because we get laxer about > accepting patches. IMO we are probably too lax already. Really? Hmm, that's not good. I seem to recall 8.4.x being pretty bad, and some of the recent bugs we fixed were actually 9.1.x problems that slipped through the cracks. > For a very long time we've tried to encourage people to submit rough > ideas to pgsql-hackers for discussion *before* they start coding. > The point of that is to weed out, or fix if possible, (some of) the bad > ideas before a lot of effort has gotten expended on them. It seems > though that less and less of that is actually happening, so I wonder > whether the commitfest process is encouraging inefficiency by making > people think they should start a discussion with a mostly-complete > patch. Or maybe CF review is just crowding out any serious discussion > of rough ideas. There was some discussion at the last dev meeting of > creating a "design review" process within commitfests, but nothing got > done about it ... I think there's been something of a professionalization of PostgreSQL development over the last few years. More and more people are able to get paid to work on PostgreSQL as part or in a few cases all of their job. This trend includes me, but also a lot of other people. There are certainly good things about this, but I think it increases the pressure to get patches committed. If you are developing a patch on your own time and it doesn't go in, it may be annoying but that's about it. If you're getting paid to get that patch committed and it doesn't go in, either your boss cares or your customer cares, or both, so now you have a bigger problem. Of course, this isn't always true: I don't know everyone's employment arrangements, but there are some people who are paid to work on PostgreSQL with no real specific agenda, just a thought of generally contributing to the community. However, I believe this to be less common than an arrangement involving specific deliverables. Whatever the arrangement, jobs where you get to work on PostgreSQL as part of your employment mean that you can get more stuff done. Whatever you can get done during work time plus any nonwork time you care to contribute will be more than what you could get done in the latter time alone. And I think we're seeing that, too. That then puts more pressure on the people who need to do reviews and commits, because there's just more stuff to look at, and you know, a lot of it is really good stuff. A lot of it has big problems, too, but we could be doing a lot worse. Nonwithstanding, it's a lot of work, and the number of people who know the system well enough to review the really difficult patches is growing, but not as fast as the number of people who have time to write them. For all of that, I'm not sure that people failing to seek consensus before coding is really so much of a problem as you seem to think. A lot of the major efforts underway for this release have been discussed extensively on multiple threads. The fact that some of those ideas may be less than half-baked at this point may in some cases be the submitter's fault, but there also cases where it's just that they haven't got enough looking-at from the people who know enough to evaluate them in detail, and perhaps some cases that are really nobody's fault: nothing in life is going to be perfect all the time no matter how hard everyone tries. -- 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] Prepared statements fail after schema changes with surprising error
Robert Haas writes: > I agree with that, but I think Tom's concern is more with the cost of > too-frequent re-planning. The most obvious case in which DDL might be > frequent enough to cause an issue here is if there is heavy use of > temporary objects - sessions might be rapidly creating and dropping > objects in their own schemas. It would be unfortunate if that forced > continual replanning of queries in other sessions. I think there > could be other cases where this is an issue as well, but the > temp-object case is probably the one that's most likely to matter in > practice. Yeah, that is probably the major hazard IMO too. The designs sketched in this thread would be sufficient to ensure that DDL in one session's temp schema wouldn't have to invalidate plans in other sessions --- but is that good enough? Your point that the locking code doesn't quite cope with newly-masked objects makes me feel that we could get away with not solving the case for plan caching either. Or at least that we could put off the problem till another day. If we are willing to just change plancache's handling of search_path, that's a small patch that I think is easily doable for 9.3. If we insist on installing schema-level invalidation logic, it's not happening before 9.4. 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] Re: Proposal for Allow postgresql.conf values to be changed via SQL [review]
On Wed, Jan 23, 2013 at 6:18 PM, Amit Kapila wrote: > On Tuesday, January 22, 2013 10:14 PM Fujii Masao wrote: >> When I removed postgresql.auto.conf and restarted the server, >> I got the following warning message. This is not correct because >> I didn't remove "auto.conf.d" from postgresql.conf. What I removed >> is only postgresql.auto.conf. >> >> WARNING: Default "auto.conf.d" is not present in postgresql.conf. >> Configuration parameters changed with SET PERSISTENT command will not >> be effective. > > How about changing it to below message: > > WARNING: File 'postgresql.auto.conf' is not accessible, either file > 'postgresql.auto.conf' or folder '%s' doesn't exist or default "auto.conf.d" > is not present in postgresql.conf. > Configuration parameters changed with SET PERSISTENT command will not be > effective. Or we should suppress such a warning message in the case where postgresql.auto.conf doesn't exist? SET PERSISTENT creates that file automatically if it doesn't exist. So we can expect that configuration parameters changed with SET PERSISTENT WILL be effective. This warning message implies that the line "include_dir 'auto.conf.d'" must not be removed from postgresql.conf? If so, we should warn that in both document and postgresql.conf.sample? Or we should hard-code so that something like auto.conf.d is always included even when that include_dir directive doesn't exist? 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] Prepared statements fail after schema changes with surprising error
2013/1/23 Robert Haas : > On Wed, Jan 23, 2013 at 8:10 AM, Dimitri Fontaine > wrote: >> Really, live DDL is not that frequent, and when you do that, you want >> transparent replanning. I can't see any use case where it's important to >> be able to run DDL in a live application yet continue to operate with >> the old (and in cases wrong) plans. > > I agree with that, but I think Tom's concern is more with the cost of > too-frequent re-planning. The most obvious case in which DDL might be > frequent enough to cause an issue here is if there is heavy use of > temporary objects - sessions might be rapidly creating and dropping > objects in their own schemas. It would be unfortunate if that forced > continual replanning of queries in other sessions. I think there > could be other cases where this is an issue as well, but the > temp-object case is probably the one that's most likely to matter in > practice. probably our model is not usual, but probably not hard exception almost all queries that we send to server are CREATE TABLE cachexxx AS SELECT ... Tables are dropped, when data there are possibility so containing data are invalid. Probably any replanning based on DDL can be very problematic in our case. Number of tables in one database can be more than 100K. Regards Pavel > > -- > 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 -- 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] pg_isready (was: [WIP] pg_ping utility)
On Mon, Jan 21, 2013 at 12:23 PM, Phil Sorber wrote: > Changing up the subject line because this is no longer a work in > progress nor is it pg_ping anymore. OK, I committed this. However, I have one suggestion. Maybe it would be a good idea to add a -c or -t option that sets the connect_timeout parameter. Because: [rhaas pgsql]$ pg_isready -h www.google.com -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] PQping Docs
On Wed, Jan 23, 2013 at 10:59 AM, Phil Sorber wrote: > On Wed, Jan 23, 2013 at 10:37 AM, Robert Haas wrote: >> On Mon, Jan 21, 2013 at 1:45 PM, Phil Sorber wrote: >>> Attached is a patch that adds a note about the FATAL messages that >>> appear in the logs if you don't pass a valid user or dbname to PQping >>> or PQpingParams. >>> >>> This was requested in the pg_isready thread. >> >> Can I counter-propose the attached, which puts the relevant text >> closer to the scene of the crime? >> > This seems reasonable to me. OK, done. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] PQping Docs
On Wed, Jan 23, 2013 at 10:37 AM, Robert Haas wrote: > On Mon, Jan 21, 2013 at 1:45 PM, Phil Sorber wrote: >> Attached is a patch that adds a note about the FATAL messages that >> appear in the logs if you don't pass a valid user or dbname to PQping >> or PQpingParams. >> >> This was requested in the pg_isready thread. > > Can I counter-propose the attached, which puts the relevant text > closer to the scene of the crime? > This seems reasonable to me. > -- > 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] Re: Proposal for Allow postgresql.conf values to be changed via SQL [review]
On Wed, Jan 23, 2013 at 12:24 PM, Amit Kapila wrote: > On Tuesday, January 22, 2013 8:25 PM Fujii Masao wrote: >> On Tue, Jan 22, 2013 at 11:07 PM, Amit Kapila >> wrote: >> > On Tuesday, January 22, 2013 7:10 PM Zoltán Böszörményi wrote: >> >> 2013-01-22 13:32 keltezéssel, Amit kapila írta: >> >> > On Saturday, January 19, 2013 2:37 AM Boszormenyi Zoltan wrote: >> >> > 2013-01-18 21:48 keltezéssel, Boszormenyi Zoltan írta: >> >> >> 2013-01-18 21:32 keltezéssel, Tom Lane írta: >> >> >>> Boszormenyi Zoltan writes: >> >> 2013-01-18 11:05 keltezéssel, Amit kapila írta: >> >> >> On using mktemp, linux compilation gives below warning >> >> >> warning: the use of `mktemp' is dangerous, better use >> `mkstemp' >> >> > >> >> Everywhere else that we need to do something like this, we just >> >> use our >> >> own PID to disambiguate, ie >> >> sprintf(tempfilename, "/path/to/file.%d", (int) getpid()); >> >> There is no need to deviate from that pattern or introduce >> >> portability >> >> issues, since we can reasonably assume that no non-Postgres >> >> programs are >> >> creating files in this directory. >> >> >>> Thanks for the enlightenment, I will post a new version soon. >> >> >> Here it is. >> >> > The patch sent by you works fine. >> >> > It needs small modification as below: >> >> > >> >> > The "auto.conf.d" directory should follow the postgresql.conf file >> >> directory not the data_directory. >> >> > The same is validated while parsing the postgresql.conf >> configuration >> >> file. >> >> > >> >> > Patch is changed to use the postgresql.conf file directory as >> below. >> >> > >> >> > StrNCpy(ConfigFileDir, ConfigFileName, sizeof(ConfigFileDir)); >> >> > get_parent_directory(ConfigFileDir); >> >> > /* Frame auto conf name and auto conf sample temp file name */ >> >> > snprintf(AutoConfFileName, sizeof(AutoConfFileName), "%s/%s/%s", >> >> > ConfigFileDir, >> >> > PG_AUTOCONF_DIR, >> >> > PG_AUTOCONF_FILENAME); >> >> >> >> Maybe it's just me but I prefer to have identical >> >> settings across all replicated servers. But I agree >> >> that there can be use cases with different setups. >> >> >> >> All in all, this change makes it necessary to run the >> >> same SET PERSISTENT statements on all slave servers, >> >> too, to make them identical again if the configuration >> >> is separated from the data directory (like on Debian >> >> or Ubuntu using the default packages). This needs to be >> >> documented explicitly. >> > >> > SET PERSISTENT command needs to run on all slave servers even if the >> path we >> > take w.r.t data directory >> > unless user takes backup after command. >> >> Is it safe to write something in the directory other than data >> directory >> via SQL? >> >> postgres user usually has the write permission for the configuration >> directory like /etc/postgresql? > > As postgresql.conf will also be in same path and if user can change that, > then what's the problem with postgresql.auto.conf If the configuration directory like /etc is owned by root and only root has a write permission for it, the user running PostgreSQL server would not be able to update postgresql.auto.conf there. OTOH, even in that case, a user can switch to root and update the configuration file there. I'm not sure whether the configuration directory is usually writable by the user running PostgreSQL server in Debian or Ubuntu, though. > Do you see any security risk? I have no idea. I just wondered that because some functions like pg_file_write() in adminpack are restricted to write something in the directory other than $PGDATA. >> >> > This closes all comments raised till now for this patch. >> >> > Kindly let me know if you feel something is missing? >> >> >> >> I can't think of anything else. >> >> For removing >> + a configuration entry from >> postgresql.auto.conf file, >> + use RESET PERSISTENT. The values will be >> effective >> + after reload of server configuration (SIGHUP) or server startup. >> >> The description of RESET PERSISTENT is in the document, but it >> seems not to be implemented. > > This command support has been removed from patch due to parser issues, so it > should be removed from documentation as well. I shall fix this along with > other issues raised by you. Okay. 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] [PATCH] PQping Docs
On Mon, Jan 21, 2013 at 1:45 PM, Phil Sorber wrote: > Attached is a patch that adds a note about the FATAL messages that > appear in the logs if you don't pass a valid user or dbname to PQping > or PQpingParams. > > This was requested in the pg_isready thread. Can I counter-propose the attached, which puts the relevant text closer to the scene of the crime? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company libpq_ping_doc_rmh.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] unlogged tables vs. GIST
On Wed, Jan 23, 2013 at 4:04 AM, Jeevan Chalke wrote: > Yes. > > I guess my earlier patch, which was directly incrementing > ControlFile->unloggedLSN counter was the concern as it will take > ControlFileLock several times. > > In this version of patch I did what Robert has suggested. At start of the > postmaster, copying unloggedLSn value to XLogCtl, a shared memory struct. > And > in all access to unloggedLSN, using this shared variable using a SpinLock. > And since we want to keep this counter persistent across clean shutdown, > storing it in ControlFile before updating it. > > With this approach, we are updating ControlFile only when we shutdown the > server, rest of the time we are having a shared memory counter. That means > we > are not touching pg_control every other millisecond or so. Also since we are > not caring about crashes, XLogging this counter like OID counter is not > required as such. On a quick read-through this looks reasonable to me, but others may have different opinions, and I haven't reviewed in detail. One obvious oversight is that bufmgr.c needs a detailed comment explaining the reason behind the change you are making there. Otherwise, someone might think to undo it in the future, and that would be bad. Perhaps add something like: However, this rule does not apply for unlogged relations, which will be lost after a crash anyway. Most unlogged relation pages do not bear LSNs since we never emit WAL records for them, and therefore flushing up through the buffer LSN would be useless, but harmless. However, GiST indexes use LSNs internally to track page-splits, and therefore temporary and unlogged GiST pages bear "fake" LSNs generated by GetXLogRecPtrForUnloggedRel. It is unlikely but possible that the fake-LSN counter used for unlogged relations could advance past the WAL insertion point; and if it did happen, attempting to flush WAL through that location would fail, with disastrous system-wide consequences. To make sure that can't happen, skip the flush if the buffer isn't permanent. A related problem is that you're examining the buffer header flags without taking the buffer-header spinlock. I'm not sure this can actually matter, because we'll always hold the content lock on the buffer at this point, which presumably precludes any operation that might flip that bit, but it looks like the callers all have the buffer header flags conveniently available, so maybe they should pass that information down to FlushBuffer. That would also avoid accessing that word in memory both before and after releasing the spinlock (all callers have just released it) which can lead to memory-access stalls. -- 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] WIP: index support for regexp search
Heikki Linnakangas writes: > On 23.01.2013 09:36, Alexander Korotkov wrote: >> On Wed, Jan 23, 2013 at 6:08 AM, Tom Lane wrote: >>> The biggest problem is that I really don't care for the idea of >>> contrib/pg_trgm being this cozy with the innards of regex_t. >> The only option I see now is to provide a method like "export_cnfa" which >> would export corresponding CNFA in fixed format. > Yeah, I think that makes sense. The transformation code in trgm_regexp.c > would probably be more readable too, if it didn't have to deal with the > regex guts representation of the CNFA. Also, once you have intermediate > representation of the original CNFA, you could do some of the > transformation work on that representation, before building the > "tranformed graph" containing trigrams. You could eliminate any > non-alphanumeric characters, joining states connected by arcs with > non-alphanumeric characters, for example. It's not just the CNFA though; the other big API problem is with mapping colors back to characters. Right now, that not only knows way too much about a part of the regex internals we have ambitions to change soon, but it also requires pg_wchar2mb_with_len() and lowerstr(), neither of which should be known to the regex library IMO. So I'm not sure how we divvy that up sanely. To be clear: I'm not going to insist that we have to have a clean API factorization before we commit this at all. But it worries me if we don't even know how we could get to that, because we are going to need it eventually. Anyway, I had another thought in the shower this morning, which is that solving this problem in terms of color trigrams is really the Wrong Thing. ISTM it'd be better to think of the CNFA traversal logic as looking for required sequences of colors of unspecified length, which we'd then chop into trigrams after the fact. This might produce slightly better (more complete) trigram sets, but the real reason I'm suggesting it is that I think the CNFA traversal code might be subject to Polya's Inventor's Paradox: "the more general problem may be easier to solve". It seems like casting the goal of that code as being to find variable-length sequences, rather than exactly trigrams, might lead to simpler data structures and more readable algorithms. The still-to-be-designed regex API for this also seems like it would be better off if decoupled from the notion of trigrams. It's quite possible that this idea is all wet and no meaningful improvement can be gotten this way. But I offer it for your consideration. 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] logical changeset generation v4
On 2013-01-23 10:18:50 -0500, Robert Haas wrote: > On Wed, Jan 23, 2013 at 7:14 AM, Andres Freund wrote: > > With the (attached for convenience) patch applied you can do > > # ALTER TABLE replication_metadata SET (treat_as_catalog_table = true); > > > > to enable this. > > What I wonder about is: > > * does anybody have a better name for the reloption? > > IMHO, it should somehow involve the words "logical" and "replication". Not a bad point. In the back of my mind I was thinking of reusing it to do error checking when accessing the heap via index methods as a way of making sure index support writers are aware of the complexities of doing so (c.f. ALTER TYPE .. ADD VALUE only being usable outside transactions). But thats probably over the top. 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] logical changeset generation v4
On Wed, Jan 23, 2013 at 7:14 AM, Andres Freund wrote: > With the (attached for convenience) patch applied you can do > # ALTER TABLE replication_metadata SET (treat_as_catalog_table = true); > > to enable this. > What I wonder about is: > * does anybody have a better name for the reloption? IMHO, it should somehow involve the words "logical" and "replication". -- 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
