Re: [HACKERS] Review of Row Level Security
On 23 December 2012 18:49, Simon Riggs si...@2ndquadrant.com wrote: Anyway, hope you can make call on 28th so we can discuss this and agree a way forwards you're happy with. Stephen, KaiGai and myself met by phone on 28th to discuss. 1. The actual default is not that important to any of us. We could go either way, or have no default at all. 2. What we do want is a declarative way of specifying row security, with options to support all use cases discussed/requested on list. We shouldn't support just one of those use cases and force everybody else to use triggers manually for the other cases. 3. We want to have the possibility of multiple row security expressions, defined for different privilege types (SELECT, UPDATE, INSERT, DELETE). (Note that this means you'd be able to specify that an update could read a row in one security mode by setting SELECT, then update that row to a new security mode by setting a clause on UPDATE - hence we refer to those as privileges not commands/events). The expressions should be separate so they can be pushed easily into query plans (exactly as in the current patch). Stephen has updated the Wiki with some ideas on how that can be structured https://wiki.postgresql.org/wiki/RLS 4. Supporting multiple expressions may not be possible for 9.3, but if not, we want to agree now what the syntax is to make sure we have a clear route for future development. If we can agree this quickly we increase the chances of KaiGai successfully implementing that. -- 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
[HACKERS] Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used
Pavel,
* Pavel Stehule (pavel.steh...@gmail.com) wrote:
A result from ours previous talk was a completely disabling mixing
positional and ordered placeholders - like is requested by man and gcc
raises warnings there.
But mixing is not explicitly disallowed in doc, and mixing was tested
in our regress tests. There are tests where placeholders are mixed -
so anybody can use it.
select format('Hello %s %1$s %s', 'World', 'Hello again'); -- is
enabled and supported and result is expected
Alright, then I agree that raising a warning in that case makes sense
and let's update the docs to reflect that it shouldn't be done (like
what glibc/gcc do).
Thanks,
Stephen
signature.asc
Description: Digital signature
[HACKERS] Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used
2012/12/31 Stephen Frost sfr...@snowman.net:
Pavel,
* Pavel Stehule (pavel.steh...@gmail.com) wrote:
A result from ours previous talk was a completely disabling mixing
positional and ordered placeholders - like is requested by man and gcc
raises warnings there.
But mixing is not explicitly disallowed in doc, and mixing was tested
in our regress tests. There are tests where placeholders are mixed -
so anybody can use it.
select format('Hello %s %1$s %s', 'World', 'Hello again'); -- is
enabled and supported and result is expected
Alright, then I agree that raising a warning in that case makes sense
and let's update the docs to reflect that it shouldn't be done (like
what glibc/gcc do).
ok, I prepare patch
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] Behaviour of bgworker with SIGHUP
Guillaume Lelarge wrote: Hi, Today, I tried to make fun with the new background worker processes in 9.3, but I found something disturbing, and need help to go further. Thanks. Is it the work of the function (pointed by bgw_sighup) to get the new config values from the postmaster? and if so, how can I get these new values? You probably want to have the sighup handler set a flag, and then call ProcessConfigFile(PGC_SIGHUP) in your main loop when the flag is set. Search for got_SIGHUP in postgres.c. I think this (have a config option, and have SIGHUP work as expected) would be useful to demo in worker_spi, if you care to submit a patch. I thought the configuration reloading would work just like a shared library but it doesn't seem so. Yeah, you need to handle that manually, because you're running your own process 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] Making view dump/restore safe at the column-alias level
On Mon, Dec 31, 2012 at 12:21 AM, Tom Lane t...@sss.pgh.pa.us wrote: On the whole I think this is a must fix bug, so we don't have a lot of choice, unless someone has a proposal for a different and more compact way of solving the problem. The only more compact way of handling things that I can see is adding syntax to let us explicitly select exactly the columns we need. But then the resulting view definitions would be Postgres-specific instead of standard SQL which would defeat a large part of the motivation to going to such lengths. I do wonder whether the SQL standard will do something obtuse enough that that's the only option for a large swathe of queries. Or is that the case already? The query syntax you're using here, is it standard SQL? Is it widely supported? -- greg -- 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] Making view dump/restore safe at the column-alias level
Greg Stark st...@mit.edu writes: I do wonder whether the SQL standard will do something obtuse enough that that's the only option for a large swathe of queries. Or is that the case already? The query syntax you're using here, is it standard SQL? Is it widely supported? Yeah, it's standard --- there's nothing here that wasn't in SQL92. (Although I notice that SQL still hasn't got any ALTER TABLE RENAME command, much less a column rename command. I wonder whether the committee is aware of these difficulties and has shied away from adding RENAME because of them?) As for widely supported, I can't imagine that the big boys don't have this, although a quick test shows that mysql only has table aliases not column aliases, ie you can do FROM t1 AS t1x but not FROM t1 AS t1x(y). Still, if that's a consideration, inventing our own syntax would be even further away from the goal. Also, the patch goes to some lengths to not print column aliases unnecessarily --- in fact, there are cases where the old code would print column aliases but the patch will not. 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] Behaviour of bgworker with SIGHUP
On Mon, 2012-12-31 at 11:03 -0300, Alvaro Herrera wrote: Guillaume Lelarge wrote: Hi, Today, I tried to make fun with the new background worker processes in 9.3, but I found something disturbing, and need help to go further. Thanks. Is it the work of the function (pointed by bgw_sighup) to get the new config values from the postmaster? and if so, how can I get these new values? You probably want to have the sighup handler set a flag, and then call ProcessConfigFile(PGC_SIGHUP) in your main loop when the flag is set. Search for got_SIGHUP in postgres.c. Thanks for the tip. It works great. I think this (have a config option, and have SIGHUP work as expected) would be useful to demo in worker_spi, if you care to submit a patch. Yeah, I would love too. Reading the code of worker_spi, we could add one or three parameters: a naptime, and the schemaname for both bgprocess. One would be enough or do you prefer all three? I thought the configuration reloading would work just like a shared library but it doesn't seem so. Yeah, you need to handle that manually, because you're running your own process now. That makes sense, thanks. -- Guillaume http://blog.guillaume.lelarge.info http://www.dalibo.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] Behaviour of bgworker with SIGHUP
Guillaume Lelarge wrote: On Mon, 2012-12-31 at 11:03 -0300, Alvaro Herrera wrote: I think this (have a config option, and have SIGHUP work as expected) would be useful to demo in worker_spi, if you care to submit a patch. Yeah, I would love too. Reading the code of worker_spi, we could add one or three parameters: a naptime, and the schemaname for both bgprocess. One would be enough or do you prefer all three? I got no problem with three. -- Á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] Behaviour of bgworker with SIGHUP
On Mon, 2012-12-31 at 12:54 -0300, Alvaro Herrera wrote: Guillaume Lelarge wrote: On Mon, 2012-12-31 at 11:03 -0300, Alvaro Herrera wrote: I think this (have a config option, and have SIGHUP work as expected) would be useful to demo in worker_spi, if you care to submit a patch. Yeah, I would love too. Reading the code of worker_spi, we could add one or three parameters: a naptime, and the schemaname for both bgprocess. One would be enough or do you prefer all three? I got no problem with three. OK, will do on wednesday. Thanks. -- Guillaume http://blog.guillaume.lelarge.info http://www.dalibo.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: optimized DROP of multiple tables within a transaction
On 30.12.2012 04:03, Robert Haas wrote: On Sun, Dec 23, 2012 at 8:41 PM, Tomas Vondra t...@fuzzy.cz wrote: Attached is a patch with fixed handling of temporary relations. I've chosen to keep the logic in DropRelFileNodeAllBuffers and rather do a local copy without the local relations. This looks pretty good, although it needs some cleanup for coding style. And I think BSEARCH_LIMIT cuts a bit too broadly as a name for the constant. I thought I followed the conding style - which guidelines have I broken? I agree that BSEARCH_LIMIT is a bit too short / not exactly descriptive. What alternative would you suggest? Granted that we can't decide on an exact value for BSEARCH_LIMIT, is there any data to indicate that 10 is at least an approximately correct value? I've presented some relevant test results on 17/12, here is the interesting part: # indexes0 1 2 3 4 5 6 789 10 11 -- unpatched 16 28 40 52 63 75 87 99 110 121 135 147 v3.1 33 43 46 56 58 60 63 72 75 76 79 80 v3.3 16 20 23 25 29 33 36 40 44 47 79 82 where v3.1 is a patch doing bsearch all the time, v3.3 uses the BSEARCH_LIMIT optimization. A few observations: (a) the v3.3 is consistently faster, and the time increases by about 3.5 second for each index (b) the v3.1 is slower at the beginning, but at the end the time increases much slower (~1.5 sec/index) (c) once we get to 4 indexes (5 relations in total), both 3.1 and 3.3 are faster than the current code (d) in case of v3.3, there's a step increase between 9 and 10 indexes (because for 10 indexes the code chooses the bsearch path), but even after this increase both versions are faster than the original code Given (b) and (c) both patches should meet at about 24 (so we might increase the BSEARCH_LIMIT e.g. to 25). It's a bit difficult to predict the behaviour on different machines (different CPUs, different amounts of shared_buffers etc.) but I guess this is safe. But even if we stick to the current value (10) I believe it's safe and both versions are much faster than the current code. regards Tomas -- 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: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used
Hello
2012/12/31 Stephen Frost sfr...@snowman.net:
Pavel,
* Pavel Stehule (pavel.steh...@gmail.com) wrote:
A result from ours previous talk was a completely disabling mixing
positional and ordered placeholders - like is requested by man and gcc
raises warnings there.
But mixing is not explicitly disallowed in doc, and mixing was tested
in our regress tests. There are tests where placeholders are mixed -
so anybody can use it.
select format('Hello %s %1$s %s', 'World', 'Hello again'); -- is
enabled and supported and result is expected
Alright, then I agree that raising a warning in that case makes sense
and let's update the docs to reflect that it shouldn't be done (like
what glibc/gcc do).
so there are two patches - first is fix in logic when positional and
ordered parameters are mixed + add warning in this situation. Second
patch enables possibility to specify width for %s conversion.
I didn't finalize documentation due my net good English skills -
probably there is necessary new paragraph about function format
elsewhere than in table
Regards
Pavel
Thanks,
Stephen
fix_mixing_positinal_ordered_placeholders.patch
Description: Binary data
format_width.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] Submission Review: User control over psql error stream
Hi Allastair, On 12/28/2012 02:33:03 PM, Alastair Turner wrote: Sorry for the slow reply ... Such is life. The discussion needs to be a little broader than stdout and stderr, there are currently three output streams from psql: - stdout - prompts, not tabular output such as the results from \set and \c - stderr - errors, notices, ... - query output - result from queries and \ commands which return tables such as \l - this is what is currently piped or redirected by \o I see a patch like this adding a fourth - diagnostic output. While this would probably be the same as stderr initially but I think that the option to make them subtly different should be kept open. What is the distinction between diagnostic output and stderr? My thought would be to distinguish between the server error stream and any errors that psql might generate. (I have some thoughts on syntax but am not yet ready to respond to your proposal.) Regards, Karl k...@meme.com Free Software: You don't pay back, you pay forward. -- Robert A. Heinlein -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] unexpected query result on hot standby server
Hello I am trying simulate hotstandby conflicts on 9.3, but without success. I have a basic hot standby configuration - on slave just hot_standby = on, standby_mode = on and primary_conninfo hot standby node works well - but I would to generate conflict I create table bubu on master and insert there value -- master create table bubu(a int); insert into bubu values(10); -- then on slave select * from bubu union all select 1 from (select pg_sleep(40)) x union all select * from bubu; -- then on master (before slave query finishing); delete from bubu; vacuum full bubu; -- this should to raise conflict, but I don't see conflict in select * from pg_stat_database_conflicts; -- query on slave returns two rows ??? postgres=# select * from bubu union all select 1 from (select pg_sleep(40)) x union all select * from bubu; a 10 1 (2 rows) strange - after setting hot_standby_feedback = on, previous query returns two rows too What I am doing wrong? Regards Pavel -- 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] fix bgworkers in EXEC_BACKEND
Heikki Linnakangas wrote:
On 27.12.2012 22:46, Alvaro Herrera wrote:
Heikki Linnakangas wrote:
Might be cleaner to directly assign the correct value to MaxBackends
above, ie. MaxBackends = MaxConnections + newval + 1 +
GetNumShmemAttachedBgworkers(). With a comment to remind that it
needs to be kept in sync with the other places where that
calculation is done, in guc.c. Or put that calculation in a new
function and call it above and in guc.c.
Thinking about this some more, it might be cleaner to move the
responsibility of setting MaxBackends out of guc.c, into
postmaster.c. The guc machinery would set max_connections and
autovacuum_max_workers as usual, but not try to set MaxBackends.
After reading the config file in postmaster.c, calculate
MaxBackends.
Actually this patch still needed one more change, because we weren't
rechecking that we're not beyond the MAX_BACKENDS value after bgworker
registration.
This is hard to hit, because with the current compile constants you need
over eight million backends in total to hit that limit (and 360 GB of
shared memory), but it seems dangerous to leave that without any
protection.
I kinda hate this a bit because I had to move MAX_BACKENDS from a
private value in guc.c to postmaster.h. But the wording of the error
message is what took the most time:
+ ereport(LOG,
+ (errcode(ERRCODE_CONFIGURATION_LIMIT_EXCEEDED),
+errmsg(too many background workers),
+errdetail(Up to %d background workers can be
registered with the current settings.,
+ MAX_BACKENDS -
(MaxConnections + autovacuum_max_workers + 1;
Completely different ideas for this error message are welcome. My first
try enumerated all the variables involved.
--
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
*** a/src/backend/postmaster/postmaster.c
--- b/src/backend/postmaster/postmaster.c
***
*** 499,504 typedef struct
--- 499,505
bool redirection_done;
bool IsBinaryUpgrade;
int max_safe_fds;
+ int MaxBackends;
#ifdef WIN32
HANDLE PostmasterHandle;
HANDLE initial_signal_pipe;
***
*** 897,911 PostmasterMain(int argc, char *argv[])
process_shared_preload_libraries();
/*
! * If loadable modules have added background workers, MaxBackends needs to
! * be updated. Do so now by forcing a no-op update of max_connections.
! * XXX This is a pretty ugly way to do it, but it doesn't seem worth
! * introducing a new entry point in guc.c to do it in a cleaner fashion.
*/
! if (GetNumShmemAttachedBgworkers() 0)
! SetConfigOption(max_connections,
! GetConfigOption(max_connections, false, false),
! PGC_POSTMASTER, PGC_S_OVERRIDE);
/*
* Establish input sockets.
--- 898,908
process_shared_preload_libraries();
/*
! * Now that loadable modules have had their chance to register background
! * workers, calculate MaxBackends. Add one for the autovacuum launcher.
*/
! MaxBackends = MaxConnections + autovacuum_max_workers + 1 +
! GetNumShmemAttachedBgworkers();
/*
* Establish input sockets.
***
*** 5214,5219 RegisterBackgroundWorker(BackgroundWorker *worker)
--- 5211,5227
return;
}
+ if (MaxConnections + autovacuum_max_workers + 1 + GetNumShmemAttachedBgworkers() =
+ MAX_BACKENDS)
+ {
+ ereport(LOG,
+ (errcode(ERRCODE_CONFIGURATION_LIMIT_EXCEEDED),
+ errmsg(too many background workers),
+ errdetail(Up to %d background workers can be registered with the current settings.,
+ MAX_BACKENDS - (MaxConnections + autovacuum_max_workers + 1;
+ return;
+ }
+
/*
* Copy the registration data into the registered workers list.
*/
***
*** 5836,5841 save_backend_variables(BackendParameters *param, Port *port,
--- 5844,5851
param-IsBinaryUpgrade = IsBinaryUpgrade;
param-max_safe_fds = max_safe_fds;
+ param-MaxBackends = MaxBackends;
+
#ifdef WIN32
param-PostmasterHandle = PostmasterHandle;
if (!write_duplicated_handle(param-initial_signal_pipe,
***
*** 6061,6066 restore_backend_variables(BackendParameters *param, Port *port)
--- 6071,6078
IsBinaryUpgrade = param-IsBinaryUpgrade;
max_safe_fds = param-max_safe_fds;
+ MaxBackends = param-MaxBackends;
+
#ifdef WIN32
Re: [HACKERS] dynamic SQL - possible performance regression in 9.2
On 12/28/12 5:11 PM, Heikki Linnakangas wrote: As it happens, I just spent a lot of time today narrowing down yet another report of a regression in 9.2, when running DBT-2: http://archives.postgresql.org/pgsql-performance/2012-11/msg7.php. It looks like that is also caused by the plancache changes. DBT-2 implements the transactions using C functions, which use SPI_execute() to run all the queries. It looks like the regression is caused by extra copying of the parse tree and plan trees. Node-copy-related functions like AllocSetAlloc and _copy* are high in the profile, They are also high in the 9.1 profile, but even more so in 9.2. I hacked together a quickdirty patch to reduce the copying of single-shot plans, and was able to buy back much of the regression I was seeing on DBT-2. Patch attached. But of course, DBT-2 really should be preparing the queries once with SPI_prepare, and reusing them thereafter. I was recently profiling an application that uses a fair amount of PL/pgSQL with dynamic queries and also noticed AllocSetAlloc high in the profile. I was getting suspicious now and compared 9.1 and 9.2 performance: 9.2 is consistently about 3% slower. Your patch doesn't seem to have a measurable effect, but it might be if I ran the test for longer. -- 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 Mon, Oct 15, 2012 at 07:53:51AM -0400, Noah Misch wrote: The only matter still requiring attention is a fix for IsoLocaleName(). Following off-list coordination with Brar, I went about finishing up this patch. The above problem proved deeper than expected. For Windows Vista, Microsoft made RFC 4646 tags the preferred way to denote a locale in Windows. Microsoft calls them locale names. Starting with Visual Studio 2012, setlocale() accepts locale names in addition to all the things it previously accepted. One can now use things like initdb --locale=zh-CN and CREATE DATABASE foo LC_CTYPE = 'pl'. This meant updating win32_langinfo() and find_matching_ts_config() to handle the new formats. In passing, I fixed an unchecked malloc() in win32_langinfo(). In addition to expanding the set of valid locale inputs, VS2012 changes the (undocumented) content of _locale_t to hold locale names where it previously held locale identifiers. I taught IsoLocaleName() to handle the new material. I also sought to improve the comments on IsoLocaleName(); its significance was not previously clear to me. This thread has some background: http://archives.postgresql.org/message-id/4964b45e.5080...@hagander.net Though I'm not entirely sanguine about digging into the officially-opaque _locale_t, we have been doing it that way for several years. None of the alternatives are clearly-satisfying. In particular, I found no good way to look up the code page corresponding to a locale name on pre-Vista systems. The CRT itself handles this by translating locale names to locale identifiers using a lookup table. The Gnulib localename and setlocale modules are also interesting studies on the topic. In previous reviews, I missed the need to update pgwin32_putenv(). The addition of VS2010 support had also missed it, so this catches up. That function has other problems, but I will hold them for another patch. Tester warning: if you currently have some form of VS2010 installed, including the compilers of Windows SDK 7.1, beware of this problem: http://stackoverflow.com/questions/10888391/link-fatal-error-lnk1123-failure-during-conversion-to-coff-file-invalid-or-c Thanks, nm *** a/doc/src/sgml/install-windows.sgml --- b/doc/src/sgml/install-windows.sgml *** *** 19,26 para There are several different ways of building PostgreSQL on productnameWindows/productname. The simplest way to build with ! Microsoft tools is to install a supported version of the ! productnameMicrosoft Windows SDK/productname and use the included compiler. It is also possible to build with the full productnameMicrosoft Visual C++ 2005, 2008 or 2010/productname. In some cases that requires the installation of the productnameWindows SDK/productname --- 19,26 para There are several different ways of building PostgreSQL on productnameWindows/productname. The simplest way to build with ! Microsoft tools is to install productnameVisual Studio Express 2012 ! for Windows Desktop/productname and use the included compiler. It is also possible to build with the full productnameMicrosoft Visual C++ 2005, 2008 or 2010/productname. In some cases that requires the installation of the productnameWindows SDK/productname *** *** 77,93 productnameVisual Studio Express/productname or some versions of the productnameMicrosoft Windows SDK/productname. If you do not already have a productnameVisual Studio/productname environment set up, the easiest ! way is to use the compilers in the productnameWindows SDK/productname, ! which is a free download from Microsoft. /para para PostgreSQL is known to support compilation using the compilers shipped with productnameVisual Studio 2005/productname to ! productnameVisual Studio 2010/productname (including Express editions), as well as standalone Windows SDK releases 6.0 to 7.1. 64-bit PostgreSQL builds are only supported with ! productnameMicrosoft Windows SDK/productname version 6.0a and above or productnameVisual Studio 2008/productname and above. /para --- 77,94 productnameVisual Studio Express/productname or some versions of the productnameMicrosoft Windows SDK/productname. If you do not already have a productnameVisual Studio/productname environment set up, the easiest ! ways are to use the compilers in the productnameWindows SDK 7.1/productname ! or those from productnameVisual Studio Express 2012 for Windows ! Desktop/productname, which are both free downloads from Microsoft. /para para PostgreSQL is known to support compilation using the compilers shipped with productnameVisual Studio 2005/productname to ! productnameVisual Studio 2012/productname (including Express editions), as well as standalone Windows SDK releases 6.0 to 7.1. 64-bit PostgreSQL builds are only supported with ! productnameMicrosoft Windows SDK/productname version
