Re: [HACKERS] PL/pgSQL, RAISE and error context
Hi here is complete patch, that introduce context filtering on client side. The core of this patch is trivial and small - almost all of size are trivial changes in regress tests - removing useless context. Documentation, check-world Regards Pavel 2015-07-26 0:42 GMT+02:00 Pavel Stehule pavel.steh...@gmail.com: Hi I am sending a next variant of filtering context patch. postgres=# do $$ begin raise notice 'kuku'; end $$; NOTICE: kuku DO Time: 2.441 ms postgres=# do $$ begin raise exception 'kuku'; end $$; ERROR: kuku CONTEXT: PL/pgSQL function inline_code_block line 1 at RAISE Time: 0.648 ms postgres=# \set SHOW_CONTEXT always postgres=# do $$ begin raise notice 'kuku'; end $$; NOTICE: kuku CONTEXT: PL/pgSQL function inline_code_block line 1 at RAISE DO Time: 0.702 ms It is a variant, when I try to filter CONTEXT in libpq. There is little bit less granularity on libpq side than server side, but still it is enough - always, error, none. This patch is without documentation, but basic regress tests works. Regards Pavel 2015-07-25 10:01 GMT+02:00 Pavel Stehule pavel.steh...@gmail.com: 2015-07-21 16:58 GMT+02:00 Merlin Moncure mmonc...@gmail.com: On Tue, Jul 21, 2015 at 2:53 AM, Heikki Linnakangas hlinn...@iki.fi wrote: On 07/21/2015 10:38 AM, Pavel Stehule wrote: where we are with this patch? Can I do some for it? I still feel this approach is misguided, and we should be tweaking psql and/or libpq instead. I don't feel strongly though, and if some other committer wants to pick this up in its current form, I won't object. So this patch has reached an impasse, and if no-one else wants to pick this up, I'm going to mark this as Returned with Feedback and move on. That's unfortunate. Maybe I'm missing something: What does a client side implementation offer that a server side implementation does not offer? I have not any problem to change the filtering to client side. Primary question is fix of PLpgSQL RAISE statement issue - The context field filtering is a necessary follow-up and trivial in both cases. In this case, it is acceptable for all? Regards Pavel merlin libpq-context-filter-20150726-01.patch.gz Description: GNU Zip compressed 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_dump -Fd and compression level
On 07/25/2015 02:34 AM, Marc Mamin wrote: Andrew Dunstan and...@dunslane.net writes: Hmm. Yeah. It looks like commit a7ad5cf0cfcfab8418000d652fa4f0c6ad6c8911 changed from using the default compression for libz to using the compression set in pg_dump options, which defaults to 0. This actually seems like the right thing to do, but it certainly should have been called out much more forcefully in release notes, and arguably should not have been changed in stable releases. Not sure what we do about it now. really 0? wouldn't that mean no compression at all? No, that's not right either. The default should be Z_DEFAULT_COMPRESSION, so we shouldn't actually see a difference in the default case. And it is definitely compressing some. So I'm now puzzled by what you're seeing. 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] Solaris testers wanted for strxfrm() behavior
On Wed, Jul 22, 2015 at 08:40:05AM +0200, Bjorn Munch wrote: On 22/07 02.29, Noah Misch wrote: I ran this program on Solaris 9 U5 (September 2006) on Sparc and got: I appreciate your testing. A few sources give December 2003 as the month for Solaris 9 Update 5; would you verify the vintage you used? Sorry I was mis-parsing the /etc/release. 9/05 is the month. I should know that. :-/ This was Solaris 9 U9 from September 2005. Does it have recent Solaris 9 updates, or is it closer to a base install of the 9/05 packages? Apparently, Oracle issued a final batch of Solaris 9 updates in February 2015. I would find it useful to know the age of the installed package containing /lib/libc.so.1. From the output it looks like the bug was present here? Yes. I did a quick search for bugs in libc with strxfrm in the title and got a few hits but none that seemed to be this one. I was curious about that. Thanks for looking. -- 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] pgbench stats per script other stuff
I liked @ because it makes sense to read it as the word at. Yep, why not. Prepending classic to the names does not look necessary. I would suggest tpcb-like, simple-update select-only, or even maybe any prefix. If the bench scripts could be read from some pg directory instead of being actually inlined, even more code could be dropped from pgbench. I think including classic would be a very good idea. Hmm. This is the command line, you have to type them! With a prefix-based approach this suggests that the builtin names must start differently so as to be easily selected. We might want to add a TPC-C like workload in the future, or any number of other things. Naming things in a good way from the outset can only make that easier. Here is a v4 which: - removes -w stuff - enhance -f with @weight - adds -b/--builtin name@weight, based on prefix builtin names are: tpcb-like, simple-update select-only, which matches their more or less historical names (although I wasn't sure of tpcb-sort-of, so I put tpcb-like) - removes -B (now can be accessed with -b tpcb-like) Pgbench builtin scripts are still inlined in the code, not in a separate directory, which might be an option to simplify the code and allow easy extensions. I still think that the --per-script-stats option is useless and per script stats should always be on as soon as several scripts are running. Even more, I think that stats (maybe no per-command stat though) should always be collected. The point of pgbench is to collect data, and the basic end-of-run tps summary is very terse and does not reflect much of what happened during the run. Also, maybe per-command detailed stats should use the same common struct to hold data as all other stats. I did not change it because it is maintained in a different part of the code. -- Fabien.diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml index 2517a3a..dc36f5d 100644 --- a/doc/src/sgml/ref/pgbench.sgml +++ b/doc/src/sgml/ref/pgbench.sgml @@ -260,7 +260,22 @@ pgbench optional replaceableoptions/ /optional replaceabledbname/ applicationpgbench/application accepts the following command-line benchmarking arguments: -variablelist + termoption-b/ replaceablescriptname[@weight]//term + termoption--builtin/ replaceablescriptname[@weight]//term + listitem + para +Add the specified builtin script to the list of executed scripts. +An optional integer weight after literal@/ allows to adjust the +probability of drawing the test. +Available builtin scripts are: literaltpcb-like/, +literalsimple-update/ and literalselect-only/. +The provided repleacablescriptname/ needs only to be a prefix +of the builtin name, hence literalsimp/ would be enough to select +literalsimple-update/. + /para + /listitem + /varlistentry + varlistentry termoption-c/option replaceableclients//term @@ -307,14 +322,15 @@ pgbench optional replaceableoptions/ /optional replaceabledbname/ /varlistentry varlistentry - termoption-f/option replaceablefilename//term - termoption--file=/optionreplaceablefilename//term + termoption-f/ replaceablefilename[@weight]//term + termoption--file=/replaceablefilename[@weight]//term listitem para -Read transaction script from replaceablefilename/. +Add a transaction script read from replaceablefilename/ to +the list of executed scripts. +An optional integer weight after literal@/ allows to adjust the +probability of drawing the test. See below for details. -option-N/option, option-S/option, and option-f/option -are mutually exclusive. /para /listitem /varlistentry @@ -404,10 +420,7 @@ pgbench optional replaceableoptions/ /optional replaceabledbname/ termoption--skip-some-updates/option/term listitem para -Do not update structnamepgbench_tellers/ and -structnamepgbench_branches/. -This will avoid update contention on these tables, but -it makes the test case even less like TPC-B. +Shorthand for option-b simple-update@1/. /para /listitem /varlistentry @@ -499,9 +512,9 @@ pgbench optional replaceableoptions/ /optional replaceabledbname/ Report the specified scale factor in applicationpgbench/'s output. With the built-in tests, this is not necessary; the correct scale factor will be detected by counting the number of -rows in the structnamepgbench_branches/ table. However, when testing -custom benchmarks (option-f/ option), the scale factor -will be reported as 1 unless this option is used. +rows in the structnamepgbench_branches/ table. +However, when testing only custom benchmarks (option-f/ option), +the scale factor will be reported
Re: [HACKERS] PL/pgSQL, RAISE and error context
2015-07-21 16:58 GMT+02:00 Merlin Moncure mmonc...@gmail.com: On Tue, Jul 21, 2015 at 2:53 AM, Heikki Linnakangas hlinn...@iki.fi wrote: On 07/21/2015 10:38 AM, Pavel Stehule wrote: where we are with this patch? Can I do some for it? I still feel this approach is misguided, and we should be tweaking psql and/or libpq instead. I don't feel strongly though, and if some other committer wants to pick this up in its current form, I won't object. So this patch has reached an impasse, and if no-one else wants to pick this up, I'm going to mark this as Returned with Feedback and move on. That's unfortunate. Maybe I'm missing something: What does a client side implementation offer that a server side implementation does not offer? I have not any problem to change the filtering to client side. Primary question is fix of PLpgSQL RAISE statement issue - The context field filtering is a necessary follow-up and trivial in both cases. In this case, it is acceptable for all? Regards Pavel merlin
Re: [HACKERS] proposal: multiple psql option -c
2015-07-23 17:52 GMT+02:00 Robert Haas robertmh...@gmail.com: On Fri, Jul 17, 2015 at 12:36 AM, Pavel Stehule pavel.steh...@gmail.com wrote: Or just properly understand the ; ? -c select * from foo; update bar set baz = 'bing'; vacuum bar; there is a risk of compatibility issues - all statements runs under one transaction implicitly So what? [pavel@dhcppc2 ~]$ psql -c insert into x values(txid_current()::text);insert into x values(txid_current()::text) postgres INSERT 0 1 [pavel@dhcppc2 ~]$ psql postgres -c select * from x a -- 1888 1888 (2 rows) I would to run -c command in separate transactions (when option --single-transaction is not used). Then is possible run -c select pg_reset ...() -c vacuum analyze ... Regards Pavel p.s. the state string INSERT 0 1 is buggy probably -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
[HACKERS] proposal: multiple psql option -c
On Saturday, July 25, 2015, Pavel Stehule pavel.steh...@gmail.com wrote: 2015-07-23 17:52 GMT+02:00 Robert Haas robertmh...@gmail.com: On Fri, Jul 17, 2015 at 12:36 AM, Pavel Stehule pavel.steh...@gmail.com wrote: Or just properly understand the ; ? -c select * from foo; update bar set baz = 'bing'; vacuum bar; there is a risk of compatibility issues - all statements runs under one transaction implicitly So what? [pavel@dhcppc2 ~]$ psql -c insert into x values(txid_current()::text);insert into x values(txid_current()::text) postgres INSERT 0 1 the state string INSERT 0 1 is buggy probably How do you figure? The last statement only inserted one record. To that point would you expect each separate -c to output its results to the console? David J.
Re: [HACKERS] optimizing vacuum truncation scans
On 22 July 2015 at 14:59, Robert Haas robertmh...@gmail.com wrote: Urgh. So if we do this, that forever precludes having HOT pruning set the all-visible bit. What is the reason why we don't do that already? Surely its a one liner? -- Simon Riggshttp://www.2ndQuadrant.com/ http://www.2ndquadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training Services
Re: [HACKERS] proposal: multiple psql option -c
2015-07-25 10:33 GMT+02:00 David G. Johnston david.g.johns...@gmail.com: On Saturday, July 25, 2015, Pavel Stehule pavel.steh...@gmail.com wrote: 2015-07-23 17:52 GMT+02:00 Robert Haas robertmh...@gmail.com: On Fri, Jul 17, 2015 at 12:36 AM, Pavel Stehule pavel.steh...@gmail.com wrote: Or just properly understand the ; ? -c select * from foo; update bar set baz = 'bing'; vacuum bar; there is a risk of compatibility issues - all statements runs under one transaction implicitly So what? [pavel@dhcppc2 ~]$ psql -c insert into x values(txid_current()::text);insert into x values(txid_current()::text) postgres INSERT 0 1 the state string INSERT 0 1 is buggy probably How do you figure? The last statement only inserted one record. I understand now, it consistent with current design. So from this view it is not error. To that point would you expect each separate -c to output its results to the console? It will be nice side effect, but my primary problem was a impossibility to combine VACUUM and any other statement to one simple psql call. Pavel David J.
Re: [HACKERS] MultiXact member wraparound protections are now enabled
On 22 July 2015 at 21:45, Robert Haas robertmh...@gmail.com wrote: But it seemed to me that this could be rather confusing. I thought it would be better to be explicit about whether the protections are enabled in all cases. That way, (1) if you see the message saying they are enabled, they are enabled; (2) if you see the message saying they are disabled, they are disabled; and (3) if you see neither message, your version does not have those protections. (3) would imply that we can't ever remove the message, in case people think they are unprotected. If we display (1) and then we find a further bug, where does that leave us? Do we put a second really, really fixed message? AIUI this refers to a bug fix, its not like we've invented some anti-virus mode to actively prevent or even scan for further error. I'm not sure why we need a message to say a bug fix has been applied; that is what the release notes are for. If something is disabled, we should say so, but otherwise silence means safety and success. -- Simon Riggshttp://www.2ndQuadrant.com/ http://www.2ndquadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training Services
Re: [HACKERS] Proposal for CSN based snapshots
On 24 July 2015 at 19:21, Robert Haas robertmh...@gmail.com wrote: On Fri, Jul 24, 2015 at 1:00 PM, Simon Riggs si...@2ndquadrant.com wrote: It depends on the exact design we use to get that. Certainly we do not want them if they cause a significant performance regression. Yeah. I think the performance worries expressed so far are: - Currently, if you see an XID that is between the XMIN and XMAX of the snapshot, you hit CLOG only on first access. After that, the tuple is hinted. With this approach, the hint bit doesn't avoid needing to hit CLOG anymore, because it's not enough to know whether or not the tuple committed; you have to know the CSN at which it committed, which means you have to look that up in CLOG (or whatever SLRU stores this data). Heikki mentioned adding some caching to ameliorate this problem, but it sounds like he was worried that the impact might still be significant. This seems like the heart of the problem. Changing a snapshot from a list of xids into one number is easy. Making XidInMVCCSnapshot() work is the hard part because there needs to be a translation/lookup from CSN to determine if it contains the xid. That turns CSN into a reference to a cached snapshot, or a reference by which a snapshot can be derived on demand. - Mixing synchronous_commit=on and synchronous_commit=off won't work as well, because if the LSN ordering of commit records matches the order in which transactions become visible, then an async-commit transaction can't become visible before a later sync-commit transaction. I expect we might just decide we can live with this, but it's worth discussing in case people feel otherwise. Using the Commit LSN as CSN seems interesting, but it is not the only choice. Commit LSN is not the precise order in which commits become visible because of the race condition between marking commit in WAL and marking commit in clog. That problem is accentuated by mixing async and sync, but that is not the only source of racing. -- Simon Riggshttp://www.2ndQuadrant.com/ http://www.2ndquadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training Services
[HACKERS] Debugging buildfarm pg_upgrade check failures
Now that we've restored proper logging of make check, I looked into today's failure report from axolotl: http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=axolotldt=2015-07-24%2020%3A29%3A18 What evidently happened there is that pg_ctl start gave up waiting for the postmaster to start too soon. The postmaster log appears to contain LOG: database system was shut down at 2015-07-24 16:45:40 EDT FATAL: the database system is starting up LOG: MultiXact member wraparound protections are now enabled LOG: database system is ready to accept connections which indicates that it did successfully come up, but not till after one PQping probe from pg_ctl, which was rejected with still starting up. Meanwhile we've got this log output from pg_ctl: waiting for server to start stopped waiting pg_ctl: could not start server Examine the log output. Counting the dots indicates that pg_ctl gave up after precisely 5 seconds. Now, looking at the logic in pg_ctl's test_postmaster_connection(), the only explanation that seems to fit the observed output is that the stat() on the postmaster pidfile (at line 650 in HEAD) failed. It's not clear why though, since the postmaster was clearly still alive at this point, and we must have been able to read the pidfile earlier to construct a connection string, else there would have been no PQping attempt. Maybe the stat failed for some unexpected resource-exhaustion kind of reason? It seems plausible to me that we should change pg_ctl to only consider stat() failure to be a reason to give up waiting if errno is ENOENT, not anything else. At a minimum, I'd like to modify it to print the errno if it's anything else, so that we can confirm or deny this theory next time we see this buildfarm failure. Comments? 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_dump -Fd and compression level
On 07/25/2015 10:50 AM, Michael Paquier wrote: On Sat, Jul 25, 2015 at 11:09 PM, Marc Mamin m.ma...@intershop.de wrote: On Sat, Jul 25, 2015 at 9:56 PM, Andrew Dunstan and...@dunslane.net wrote: On 07/25/2015 03:20 AM, Andrew Dunstan wrote: On 07/25/2015 02:34 AM, Marc Mamin wrote: Andrew Dunstan and...@dunslane.net writes: Hmm. Yeah. It looks like commit a7ad5cf0cfcfab8418000d652fa4f0c6ad6c8911 changed from using the default compression for libz to using the compression set in pg_dump options, which defaults to 0. This actually seems like the right thing to do, but it certainly should have been called out much more forcefully in release notes, and arguably should not have been changed in stable releases. Not sure what we do about it now. really 0? wouldn't that mean no compression at all? No, that's not right either. The default should be Z_DEFAULT_COMPRESSION, so we shouldn't actually see a difference in the default case. And it is definitely compressing some. So I'm now puzzled by what you're seeing. OK, I have got this worked out. I'll have a bug fix shortly. So you are basically planning to switch to Z_BEST_SPEED instead of Z_DEFAULT_COMPRESSION where needed for the default code path? It looks like Z_DEFAULT_COMPRESSION was the used value before, and from the source code I guess this should still be the case. From a quick testing, it now behaves as if the minus sign from Z_DEFAULT_COMPRESSION is lost on the way, resulting in -Z1. this might indicate that Z_DEFAULT_COMPRESSION is parsed like the command line argument. gzopen only allows a digit, so -1 results in the - being ignored and 1 being the compression value. It's a pity gzopen is so liberal about accepting modes with values it ignores, or we'd have noticed this ages ago. I think I understand what is happening... With a quick test using the default compression level in directory mode, gzopen is called with w-1. By decrypting the docs of zlib (http://www.zlib.net/manual.html), section File Access Functions, it seems to me that calling gzopen like that will cause the file to not be compressed at all, which is actually why you are seeing an increase in your dump files, while we should call it with a compression mode of 6 actually based on what Z_DEFAULT_COMPRESSION means for zlib now, or simply without a compression level specified such as the default is used by zlib. Prior to a7ad5cf0, what we did was actually gzopen with always w or wb that caused the default compression level of 6 to be used. Hence I think that we should definitely use w or wb without specifying a level number when the compression level is Z_DEFAULT_COMPRESSION, and wbN (0=N=9) when a compression level is given by the user. A patch is attached for this purpose. Marc, does it work for you? e.g. pg_dump -Z-2 is like pg_dump -Z2 The compression needs to be included in range [0,9]. I propose to tighten pg_dump's rules so that only 0..9 are accepted as arguments for -Z, and in compress_io.c:cfopen(), if compression is equal to Z_DEFAULT_COMPRESSION, not add any explicit compression value to the mode, thus using the zlib default. 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] Supporting TAP tests with MSVC and Windows
On Sat, Jul 25, 2015 at 4:14 PM, Noah Misch n...@leadboat.com wrote:
On Fri, Jul 24, 2015 at 08:27:42PM +0300, Heikki Linnakangas wrote:
On 06/25/2015 07:40 AM, Michael Paquier wrote:
Attached is v7, rebased on 0b157a0.
Thanks! I fiddled with this a bit more, to centralize more of the
platform-dependent stuff to RewindTest.pm. Also, Windows doesn't have cat
and touch if you haven't installed MinGW, so I replaced those calls with
built-in perl code.
My main priority for this patch is to not reintroduce CVE-2014-0067. The
patch is operating in a security minefield:
Thanks!
--- a/src/bin/pg_ctl/t/001_start_stop.pl
+++ b/src/bin/pg_ctl/t/001_start_stop.pl
@@ -15,13 +15,9 @@ command_exit_is([ 'pg_ctl', 'start', '-D',
$tempdir/nonexistent ],
command_ok([ 'pg_ctl', 'initdb', '-D', $tempdir/data ], 'pg_ctl initdb');
command_ok(
- [ $ENV{top_builddir}/src/test/regress/pg_regress, '--config-auth',
+ [ $ENV{PG_REGRESS}, '--config-auth',
$tempdir/data ],
'configure authentication');
-open CONF, $tempdir/data/postgresql.conf;
-print CONF listen_addresses = ''\n;
-print CONF unix_socket_directories = '$tempdir_short'\n;
-close CONF;
command_ok([ 'pg_ctl', 'start', '-D', $tempdir/data, '-w' ],
'pg_ctl start -w');
Since this series of tests doesn't use standard_initdb, the deleted lines
remain necessary.
Added with a switch on $config{osname}.
@@ -303,7 +297,6 @@ sub run_pg_rewind
}
else
{
-
# Cannot come here normally
Unrelated change.
Removed.
sub standard_initdb
{
my $pgdata = shift;
system_or_bail('initdb', '-D', $pgdata, '-A' , 'trust', '-N');
- system_or_bail($ENV{top_builddir}/src/test/regress/pg_regress,
- '--config-auth', $pgdata);
+ system_or_bail($ENV{PG_REGRESS}, '--config-auth', $pgdata);
+
+ my $tempdir_short = tempdir_short;
+
+ open CONF, $pgdata/postgresql.conf;
+ print CONF \n# Added by TestLib.pm)\n;
+ if ($Config{osname} eq MSWin32)
+ {
+ print CONF listen_addresses = '127.0.0.1'\n;
+ }
+ else
+ {
+ print CONF unix_socket_directories = '$tempdir_short'\n;
This second branch needs listen_addresses=''; it doesn't help to secure the
socket directory if the server still listens on localhost.
Yep. Agreed.
+sub configure_hba_for_replication
+{
+ my $pgdata = shift;
+
+ open HBA, $pgdata/pg_hba.conf;
+ print HBA \n# Allow replication (set up by TestLib.pm)\n;
+ if ($Config{osname} ne MSWin32)
+ {
+ print HBA local replication all trust\n;
+ }
+ else
+ {
+ print HBA host replication all 127.0.0.1/32 sspi
include_realm=1 map=regress\n;
+ print HBA host replication all ::1/128 sspi include_realm=1
map=regress\n;
The second line will make the server fail to start on non-IPv6 systems. You
shouldn't need it if the clients always connect to 127.0.0.1, not localhost.
Done.
+ configure_for_replication($tempdir/pgdata);
That function name appears nowhere else.
This looks like dead code to me. Hence removed.
+sub tapcheck
+{
+ InstallTemp();
+
+ my @args = ( prove, --verbose, t/*.pl);
+
+ $ENV{PATH} = $tmp_installdir/bin;$ENV{PATH};
+ $ENV{PERL5LIB} = $topdir/src/test/perl;$ENV{PERL5LIB};
+ $ENV{PG_REGRESS} = $topdir/$Config/pg_regress/pg_regress;
+
+ # Find out all the existing TAP tests by simply looking for t/
+ # in the tree.
This target shall not run the SSL suite, for the same reason check-world shall
not run it. We could offer a distinct vcregress.pl target for TAP suites
excluded from check-world.
OK, for the sake of duplicating what GNU does, let's simply ignore it then.
Also I added an equivalent of --enable-tap-tests for this MSVC stuff,
removed afterwards by Heikki. Do we want it or not?
An updated patch is attached.
--
Michael
From 51d6d553742753a3389fcb67778662cb24274508 Mon Sep 17 00:00:00 2001
From: Michael Paquier mich...@otacoo.com
Date: Sat, 25 Jul 2015 22:50:49 +0900
Subject: [PATCH] Make TAP tests work on Windows.
On Windows, use listen_address=127.0.0.1 to allow connections. We were
already using pg_regress --config-auth to set up HBA appropriately.
The standard_initdb helper function now sets up the server's
unix_socket_directories or listen_addresses in the config file, so that
they don't need to be specified in the pg_ctl command line anymore. That
way, the pg_ctl invocations in test programs don't need to differ between
Windows and Unix.
Add another helper function to configure the server's pg_hba.conf to
allow replication connections. The configuration is done similarly to
pg_regress --config-auth: trust on domain sockets on Unix, and SSPI
authentication on Windows.
Replace calls to cat and touch programs with built-in perl code.
Add vcregress tapcheck command for launching the tests on Windows.
Michael
Re: [HACKERS] RFC: replace pg_stat_activity.waiting with something more descriptive
On Jul 24, 2015, at 10:02 PM, Robert Haas robertmh...@gmail.com wrote: Also, the patch should not invent a new array similar but not quite identical to LockTagTypeNames[]. This is goofy: + if (tranche_id 0) + result-tranche = tranche_id; + else + result-tranche = LW_USERDEFINED; If we're going to make everybody pass a tranche ID when they call LWLockAssign(), then they should have to do that always, not sometimes pass a tranche ID and otherwise pass 0, which is actually a valid tranche ID but to which you've given a weird special meaning here. I'd also name the constants differently, like LWLOCK_TRANCHE_XXX or something. LW_ is a somewhat ambiguous prefix. The idea of tranches is really that each tranche is an array of items each of which contains 1 or more lwlocks. Here you are intermingling different tranches. I guess that won't really break anything but it seems ugly. Maybe it will be better to split LWLockAssign to two functions then, keep name LWLockAssign for user defined locks and other function with tranche_id. On Jul 25, 2015, at 1:54 PM, Amit Kapila amit.kapil...@gmail.com wrote: That anyway he has to do it either you go by defining individual arrays or having unified WaitEventType enum for individual arrays he has to find out that array. Another thing is with that you can just encapsulate this information in one byte in structure PgBackendStatus, rather than using more number of bytes (4 bytes) and I think the function for reporting Waitevent will be much more simplified. In my way there are no special meaning for names. Array with names located in lwlock.c and lock.c, and can be used for other things (for example tracing). One byte sounds good only for this case. We are going to extend waits monitoring, add more views, some profiling. That’s why waits have to be groupable by classes. Ildus Kurbangaliev Postgres Professional: http://www.postgrespro.com The Russian 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
[HACKERS] Anyone working on the TOAST items on the TODO list?
If not, I'm going to make a little personal project of them for myself (targeting 9.6). -- Bill Moran -- 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_dump -Fd and compression level
On Sat, Jul 25, 2015 at 11:09 PM, Marc Mamin m.ma...@intershop.de wrote: On Sat, Jul 25, 2015 at 9:56 PM, Andrew Dunstan and...@dunslane.net wrote: On 07/25/2015 03:20 AM, Andrew Dunstan wrote: On 07/25/2015 02:34 AM, Marc Mamin wrote: Andrew Dunstan and...@dunslane.net writes: Hmm. Yeah. It looks like commit a7ad5cf0cfcfab8418000d652fa4f0c6ad6c8911 changed from using the default compression for libz to using the compression set in pg_dump options, which defaults to 0. This actually seems like the right thing to do, but it certainly should have been called out much more forcefully in release notes, and arguably should not have been changed in stable releases. Not sure what we do about it now. really 0? wouldn't that mean no compression at all? No, that's not right either. The default should be Z_DEFAULT_COMPRESSION, so we shouldn't actually see a difference in the default case. And it is definitely compressing some. So I'm now puzzled by what you're seeing. OK, I have got this worked out. I'll have a bug fix shortly. So you are basically planning to switch to Z_BEST_SPEED instead of Z_DEFAULT_COMPRESSION where needed for the default code path? It looks like Z_DEFAULT_COMPRESSION was the used value before, and from the source code I guess this should still be the case. From a quick testing, it now behaves as if the minus sign from Z_DEFAULT_COMPRESSION is lost on the way, resulting in -Z1. this might indicate that Z_DEFAULT_COMPRESSION is parsed like the command line argument. I think I understand what is happening... With a quick test using the default compression level in directory mode, gzopen is called with w-1. By decrypting the docs of zlib (http://www.zlib.net/manual.html), section File Access Functions, it seems to me that calling gzopen like that will cause the file to not be compressed at all, which is actually why you are seeing an increase in your dump files, while we should call it with a compression mode of 6 actually based on what Z_DEFAULT_COMPRESSION means for zlib now, or simply without a compression level specified such as the default is used by zlib. Prior to a7ad5cf0, what we did was actually gzopen with always w or wb that caused the default compression level of 6 to be used. Hence I think that we should definitely use w or wb without specifying a level number when the compression level is Z_DEFAULT_COMPRESSION, and wbN (0=N=9) when a compression level is given by the user. A patch is attached for this purpose. Marc, does it work for you? e.g. pg_dump -Z-2 is like pg_dump -Z2 The compression needs to be included in range [0,9]. -- Michael diff --git a/src/bin/pg_dump/compress_io.c b/src/bin/pg_dump/compress_io.c index 912fc2f..10d52c8 100644 --- a/src/bin/pg_dump/compress_io.c +++ b/src/bin/pg_dump/compress_io.c @@ -549,8 +549,12 @@ cfopen(const char *path, const char *mode, int compression) #ifdef HAVE_LIBZ char mode_compression[32]; - snprintf(mode_compression, sizeof(mode_compression), %s%d, - mode, compression); + if (compression == Z_DEFAULT_COMPRESSION) + snprintf(mode_compression, sizeof(mode_compression), %s, + mode); + else + snprintf(mode_compression, sizeof(mode_compression), %s%d, + mode, compression); fp-compressedfp = gzopen(path, mode_compression); fp-uncompressedfp = NULL; if (fp-compressedfp == NULL) -- 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_dump -Fd and compression level
On Sat, Jul 25, 2015 at 11:09 PM, Marc Mamin m.ma...@intershop.de wrote: On Sat, Jul 25, 2015 at 9:56 PM, Andrew Dunstan and...@dunslane.net wrote: On 07/25/2015 03:20 AM, Andrew Dunstan wrote: On 07/25/2015 02:34 AM, Marc Mamin wrote: Andrew Dunstan and...@dunslane.net writes: Hmm. Yeah. It looks like commit a7ad5cf0cfcfab8418000d652fa4f0c6ad6c8911 changed from using the default compression for libz to using the compression set in pg_dump options, which defaults to 0. This actually seems like the right thing to do, but it certainly should have been called out much more forcefully in release notes, and arguably should not have been changed in stable releases. Not sure what we do about it now. really 0? wouldn't that mean no compression at all? No, that's not right either. The default should be Z_DEFAULT_COMPRESSION, so we shouldn't actually see a difference in the default case. And it is definitely compressing some. So I'm now puzzled by what you're seeing. OK, I have got this worked out. I'll have a bug fix shortly. So you are basically planning to switch to Z_BEST_SPEED instead of Z_DEFAULT_COMPRESSION where needed for the default code path? It looks like Z_DEFAULT_COMPRESSION was the used value before, and from the source code I guess this should still be the case. From a quick testing, it now behaves as if the minus sign from Z_DEFAULT_COMPRESSION is lost on the way, resulting in -Z1. this might indicate that Z_DEFAULT_COMPRESSION is parsed like the command line argument. I think I understand what is happening... With a quick test using the default compression level in directory mode, gzopen is called with w-1. By decrypting the docs of zlib (http://www.zlib.net/manual.html), section File Access Functions, it seems to me that calling gzopen like that will cause the file to not be compressed at all, which is actually why you are seeing an increase in your dump files, But it does compress. Seems to be the same as -Z1 while we should call it with a compression mode of 6 actually based on what Z_DEFAULT_COMPRESSION means for zlib now, or simply without a compression level specified such as the default is used by zlib. Prior to a7ad5cf0, what we did was actually gzopen with always w or wb that caused the default compression level of 6 to be used. Hence I think that we should definitely use w or wb without specifying a level number when the compression level is Z_DEFAULT_COMPRESSION, and wbN (0=N=9) when a compression level is given by the user. A patch is attached for this purpose. Marc, does it work for you? I'm on vacation and won't be able to test patches for the 2 next weeks. sorry e.g. pg_dump -Z-2 is like pg_dump -Z2 The compression needs to be included in range [0,9]. Yes, but pg_dump is not such strict. e.g. pg_dump -Zx is accepted and leads to no compression These 2 calls result in the same compression, which is how I came to the idea that the minus sign of Z_DEFAULT_COMPRESSION might get lost on the way: pg_dump -Z9 ... pg_dump -Z-9 ... Marc -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgbench stats per script other stuff
Also, maybe per-command detailed stats should use the same common struct to hold data as all other stats. I did not change it because it is maintained in a different part of the code. I played just once with the --report-latencies option and was astonished that meta commands showed negative latencies... This v5 also fixes this bug (on meta commands there is a goto loop in doCustom, but as now was not reset the stmt_begin ended up being after now, hence accumulating increasing negative times) and in passing uses the same stats structure as the rest, which result in removing some more code. The report-latencies option is made to imply per script stats, which simplifies the final output code, and if you want per-command per-script stats, probably providing the per-script stats, i.e. the sum of the commands, make sense. -- Fabien.diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml index 2517a3a..dc36f5d 100644 --- a/doc/src/sgml/ref/pgbench.sgml +++ b/doc/src/sgml/ref/pgbench.sgml @@ -260,7 +260,22 @@ pgbench optional replaceableoptions/ /optional replaceabledbname/ applicationpgbench/application accepts the following command-line benchmarking arguments: -variablelist + termoption-b/ replaceablescriptname[@weight]//term + termoption--builtin/ replaceablescriptname[@weight]//term + listitem + para +Add the specified builtin script to the list of executed scripts. +An optional integer weight after literal@/ allows to adjust the +probability of drawing the test. +Available builtin scripts are: literaltpcb-like/, +literalsimple-update/ and literalselect-only/. +The provided repleacablescriptname/ needs only to be a prefix +of the builtin name, hence literalsimp/ would be enough to select +literalsimple-update/. + /para + /listitem + /varlistentry + varlistentry termoption-c/option replaceableclients//term @@ -307,14 +322,15 @@ pgbench optional replaceableoptions/ /optional replaceabledbname/ /varlistentry varlistentry - termoption-f/option replaceablefilename//term - termoption--file=/optionreplaceablefilename//term + termoption-f/ replaceablefilename[@weight]//term + termoption--file=/replaceablefilename[@weight]//term listitem para -Read transaction script from replaceablefilename/. +Add a transaction script read from replaceablefilename/ to +the list of executed scripts. +An optional integer weight after literal@/ allows to adjust the +probability of drawing the test. See below for details. -option-N/option, option-S/option, and option-f/option -are mutually exclusive. /para /listitem /varlistentry @@ -404,10 +420,7 @@ pgbench optional replaceableoptions/ /optional replaceabledbname/ termoption--skip-some-updates/option/term listitem para -Do not update structnamepgbench_tellers/ and -structnamepgbench_branches/. -This will avoid update contention on these tables, but -it makes the test case even less like TPC-B. +Shorthand for option-b simple-update@1/. /para /listitem /varlistentry @@ -499,9 +512,9 @@ pgbench optional replaceableoptions/ /optional replaceabledbname/ Report the specified scale factor in applicationpgbench/'s output. With the built-in tests, this is not necessary; the correct scale factor will be detected by counting the number of -rows in the structnamepgbench_branches/ table. However, when testing -custom benchmarks (option-f/ option), the scale factor -will be reported as 1 unless this option is used. +rows in the structnamepgbench_branches/ table. +However, when testing only custom benchmarks (option-f/ option), +the scale factor will be reported as 1 unless this option is used. /para /listitem /varlistentry @@ -511,7 +524,7 @@ pgbench optional replaceableoptions/ /optional replaceabledbname/ termoption--select-only/option/term listitem para -Perform select-only transactions instead of TPC-B-like test. +Shorthand for option-b select-only@1/. /para /listitem /varlistentry @@ -567,6 +580,16 @@ pgbench optional replaceableoptions/ /optional replaceabledbname/ /varlistentry varlistentry + termoption--per-script-stats/option/term + listitem + para +Report some statistics per script run by pgbench. + /para + /listitem + /varlistentry + + + varlistentry termoption--sampling-rate=replaceablerate//option/term listitem para @@ -661,7 +684,20 @@ pgbench optional replaceableoptions/ /optional replaceabledbname/ titleWhat is the quoteTransaction/ Actually Performed in pgbench?/title
Re: [HACKERS] pg_dump quietly ignore missing tables - is it bug?
Hi I am sending a new patch - without checking wildcard chars. Regards Pavel 2015-07-23 7:22 GMT+02:00 Kyotaro HORIGUCHI horiguchi.kyot...@lab.ntt.co.jp : Hello, 2015-07-19 20:54 GMT+02:00 Pavel Stehule pavel.steh...@gmail.com: I am sending updated version. It implements new long option --strict-names. If this option is used, then for any entered name (without any wildcard char) must be found least one object. This option has not impact on patters (has wildcards chars). I share the same option with Tom that it should behave in same way regardless of the appearance of wildcards. You replied Tom as this the behave is same - only one real identifier is allowed But the description above about this patch apparently says that they are differently handled. And expand_(schema|table)_name_patterns does the further differently thing from both of Tom's suggestion and what mentioned in your reply to that. I will mention for this topic again in this mail. # Might only one real ident is allowed mean at most one # match, but not exactly one match? They do like this when strict-names. - Not allow no match for non-wildcarded names. - Allow no match for any wildcarded name spec and finally allowing *all* of them don't match anyting. This looks to me quite confusing. When this option is not used, then behave is 100% same as before (with same numbers of SQL queries for -t option). It is based on Oleksandr's documentation (and lightly modified philosophy), and cleaned my previous patch. A test on wildchard existency strcspn(cell-val, ?*) cannot be used, because it doesn't calculate quotes (but a replace has few lines only). The new name processSQLName looks a bit bogus. processSQLNameIntenral would be a name commonly seen in such cases. There is a possibility to remove a wildcard char test and require least one entry for patters too. But I am thinking, when somebody explicitly uses any wildcard, then he calculate with a possibility of empty result. Why do you think so? Wild cards are usually used to glob multiple names at once. One or more matches are expected for many or perhaps most cases, I think. Since so, if someone anticipate that some of his patterns have no match, I think he shouldn't specify --strict-names option at all. Furthermore, I don't think no one wants to use both wildcarded and non-wildcarded name specs at once but this is a little out of scope. I'd like to have opinions from others about this point. other variant is using --strict-names behave as default (and implement negative option like --disable-strict-names or some similar). This contradicts Josh's request. (which I'm not totally agree:p) Note: originally I though, we have to fix it and change the default behave. But with special option, we don't need it. This option in help is signal for user, so some is risky. regards, -- Kyotaro Horiguchi NTT Open Source Software Center diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml new file mode 100644 index 7467e86..7c071fb *** a/doc/src/sgml/ref/pg_dump.sgml --- b/doc/src/sgml/ref/pg_dump.sgml *** PostgreSQL documentation *** 545,550 --- 545,566 /varlistentry varlistentry + termoption--strict-names//term + listitem +para + Require that table and/or schema match at least one entity each. + Without any entity in the database to be dumped, an error message + is printed and dump is aborted. +/para +para + This option has no effect on the exclude table and schema patterns + (and also option--exclude-table-data/): not matching any entities + isn't considered an error. +/para + /listitem + /varlistentry + + varlistentry termoption-T replaceable class=parametertable/replaceable/option/term termoption--exclude-table=replaceable class=parametertable/replaceable/option/term listitem diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c new file mode 100644 index 0e036b8..54618fa *** a/src/bin/pg_dump/pg_dump.c --- b/src/bin/pg_dump/pg_dump.c *** static const char *username_subquery; *** 97,102 --- 97,105 /* obsolete as of 7.3: */ static Oid g_last_builtin_oid; /* value of the last builtin oid */ + /* The specified names/patterns should to match at least one entity */ + static int strict_mode = 0; + /* * Object inclusion/exclusion lists * *** static void setup_connection(Archive *AH *** 131,140 static ArchiveFormat parseArchiveFormat(const char *format, ArchiveMode *mode); static void expand_schema_name_patterns(Archive *fout, SimpleStringList *patterns, ! SimpleOidList *oids); static void expand_table_name_patterns(Archive *fout, SimpleStringList *patterns, ! SimpleOidList *oids);
Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.
2015-07-25 0:41 GMT+02:00 Josh Berkus j...@agliodbs.com: On 07/24/2015 11:06 AM, Jim Nasby wrote: On 7/23/15 5:18 AM, Thakur, Sameer wrote: Hello, logged 25 times Sorry, it is much lower at 7 times. Does not change overall point though I think it's related to the problem of figuring out how many dead tuples you expect to find in the overall heap, which you need to do to have any hope of this being a comprehensive estimate. What about just reporting scanned pages/total pages ? That would be easy and cheap to track. It would result in some herky-jerky progress, but would still be an improvement over the feedback we don't have now. I like this idea. Regards Pavel -- 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] pg_dump -Fd and compression level
Andrew Dunstan and...@dunslane.net writes: Hmm. Yeah. It looks like commit a7ad5cf0cfcfab8418000d652fa4f0c6ad6c8911 changed from using the default compression for libz to using the compression set in pg_dump options, which defaults to 0. This actually seems like the right thing to do, but it certainly should have been called out much more forcefully in release notes, and arguably should not have been changed in stable releases. Not sure what we do about it now. really 0? wouldn't that mean no compression at all? I don't think we realized that we were changing the default behavior. Still, it's clearly a bug fix, so I'm disinclined to revert it now. regards, tom lane Sure, but at least the doc should be modified as it suggests that a higher compression is used: the default is to compress at a moderate level = the default is to compress at the lowest level And maybe a quick notice for planet Postgres to highlight the modification of the default behavior. (I' haven't a blog myself ...) IMHO a slightly higher default would help spare some hardware out there... regards, Marc Mamin -- 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] Supporting TAP tests with MSVC and Windows
On Fri, Jul 24, 2015 at 08:27:42PM +0300, Heikki Linnakangas wrote:
On 06/25/2015 07:40 AM, Michael Paquier wrote:
Attached is v7, rebased on 0b157a0.
Thanks! I fiddled with this a bit more, to centralize more of the
platform-dependent stuff to RewindTest.pm. Also, Windows doesn't have cat
and touch if you haven't installed MinGW, so I replaced those calls with
built-in perl code.
My main priority for this patch is to not reintroduce CVE-2014-0067. The
patch is operating in a security minefield:
--- a/src/bin/pg_ctl/t/001_start_stop.pl
+++ b/src/bin/pg_ctl/t/001_start_stop.pl
@@ -15,13 +15,9 @@ command_exit_is([ 'pg_ctl', 'start', '-D',
$tempdir/nonexistent ],
command_ok([ 'pg_ctl', 'initdb', '-D', $tempdir/data ], 'pg_ctl initdb');
command_ok(
- [ $ENV{top_builddir}/src/test/regress/pg_regress, '--config-auth',
+ [ $ENV{PG_REGRESS}, '--config-auth',
$tempdir/data ],
'configure authentication');
-open CONF, $tempdir/data/postgresql.conf;
-print CONF listen_addresses = ''\n;
-print CONF unix_socket_directories = '$tempdir_short'\n;
-close CONF;
command_ok([ 'pg_ctl', 'start', '-D', $tempdir/data, '-w' ],
'pg_ctl start -w');
Since this series of tests doesn't use standard_initdb, the deleted lines
remain necessary.
@@ -303,7 +297,6 @@ sub run_pg_rewind
}
else
{
-
# Cannot come here normally
Unrelated change.
sub standard_initdb
{
my $pgdata = shift;
system_or_bail('initdb', '-D', $pgdata, '-A' , 'trust', '-N');
- system_or_bail($ENV{top_builddir}/src/test/regress/pg_regress,
- '--config-auth', $pgdata);
+ system_or_bail($ENV{PG_REGRESS}, '--config-auth', $pgdata);
+
+ my $tempdir_short = tempdir_short;
+
+ open CONF, $pgdata/postgresql.conf;
+ print CONF \n# Added by TestLib.pm)\n;
+ if ($Config{osname} eq MSWin32)
+ {
+ print CONF listen_addresses = '127.0.0.1'\n;
+ }
+ else
+ {
+ print CONF unix_socket_directories = '$tempdir_short'\n;
This second branch needs listen_addresses=''; it doesn't help to secure the
socket directory if the server still listens on localhost.
+sub configure_hba_for_replication
+{
+ my $pgdata = shift;
+
+ open HBA, $pgdata/pg_hba.conf;
+ print HBA \n# Allow replication (set up by TestLib.pm)\n;
+ if ($Config{osname} ne MSWin32)
+ {
+ print HBA local replication all trust\n;
+ }
+ else
+ {
+ print HBA host replication all 127.0.0.1/32 sspi
include_realm=1 map=regress\n;
+ print HBA host replication all ::1/128 sspi include_realm=1
map=regress\n;
The second line will make the server fail to start on non-IPv6 systems. You
shouldn't need it if the clients always connect to 127.0.0.1, not localhost.
+ configure_for_replication($tempdir/pgdata);
That function name appears nowhere else.
+sub tapcheck
+{
+ InstallTemp();
+
+ my @args = ( prove, --verbose, t/*.pl);
+
+ $ENV{PATH} = $tmp_installdir/bin;$ENV{PATH};
+ $ENV{PERL5LIB} = $topdir/src/test/perl;$ENV{PERL5LIB};
+ $ENV{PG_REGRESS} = $topdir/$Config/pg_regress/pg_regress;
+
+ # Find out all the existing TAP tests by simply looking for t/
+ # in the tree.
This target shall not run the SSL suite, for the same reason check-world shall
not run it. We could offer a distinct vcregress.pl target for TAP suites
excluded from check-world.
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] Anyone working on the TOAST items on the TODO list?
On Sat, Jul 25, 2015 at 11:05 PM, Bill Moran wmo...@potentialtech.com wrote: If not, I'm going to make a little personal project of them for myself (targeting 9.6). If you mean those three ones, not that I know of: https://wiki.postgresql.org/wiki/Todo#TOAST -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_dump -Fd and compression level
On Sat, Jul 25, 2015 at 9:56 PM, Andrew Dunstan and...@dunslane.net wrote: On 07/25/2015 03:20 AM, Andrew Dunstan wrote: On 07/25/2015 02:34 AM, Marc Mamin wrote: Andrew Dunstan and...@dunslane.net writes: Hmm. Yeah. It looks like commit a7ad5cf0cfcfab8418000d652fa4f0c6ad6c8911 changed from using the default compression for libz to using the compression set in pg_dump options, which defaults to 0. This actually seems like the right thing to do, but it certainly should have been called out much more forcefully in release notes, and arguably should not have been changed in stable releases. Not sure what we do about it now. really 0? wouldn't that mean no compression at all? No, that's not right either. The default should be Z_DEFAULT_COMPRESSION, so we shouldn't actually see a difference in the default case. And it is definitely compressing some. So I'm now puzzled by what you're seeing. OK, I have got this worked out. I'll have a bug fix shortly. So you are basically planning to switch to Z_BEST_SPEED instead of Z_DEFAULT_COMPRESSION where needed for the default code path? -- Michael It looks like Z_DEFAULT_COMPRESSION was the used value before, and from the source code I guess this should still be the case. From a quick testing, it now behaves as if the minus sign from Z_DEFAULT_COMPRESSION is lost on the way, resulting in -Z1. this might indicate that Z_DEFAULT_COMPRESSION is parsed like the command line argument. e.g. pg_dump -Z-2 is like pg_dump -Z2 Marc -- 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] pgbench - allow backslash-continuations in custom scripts
oops, sorry, stalled post because of wrong from, posting again... Attatched is the revised version of this patch. The first patch is not changed from before. The second is fixed a kind of bug. Ths third is the new one to allow backslash continuation for backslash commands. Ah, thanks:-) Would you consider adding the patch to the next commitfest? I may put myself as a reviewer... [...] I'm not satisfied by the design but I don't see another way.. I'll try to have a look. I don't have idea how to deal with the copy of psqlscan.[lh] from psql. Currently they are simply the dead copies of those of psql. I think that there should be no copies, but it should use relative symbolic links so that the files are kept synchronized. Yeah, I think so but symlinks could harm on git and Windows. The another way would be make copies it from psql directory. They live next door to each other. Indeed there are plenty of links already which are generated by makefiles (see src/bin/pg_xlogdump/*), and probably a copy is made on windows. There should no file duplication within the source tree. -- Fabien. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] creating extension including dependencies
On 2015-07-25 14:37, Michael Paquier wrote: On Sat, Jul 25, 2015 at 12:59 AM, Petr Jelinek p...@2ndquadrant.com wrote: On 2015-07-22 07:12, Michael Paquier wrote: On Tue, Jul 21, 2015 at 11:20 PM, Tom Lane t...@sss.pgh.pa.us wrote: Petr Jelinek p...@2ndquadrant.com writes: ... My main question is if we are ok with SCHEMA having different behavior with CASCADE vs without CASCADE. I went originally with no and added the DEFAULT flag to SCHEMA. If the answer is yes then we don't need the flag (in that case CASCADE acts as the flag). Yeah, I was coming around to that position as well. Insisting that SCHEMA throw an error if the extension isn't relocatable makes sense as long as only one extension is being considered, but once you say CASCADE it seems like mostly a usability fail. I think it's probably OK if with CASCADE, SCHEMA is just use if needed else ignore. Here is a patch implementing that... + para + If schema is not same as the one in extension's control file and + the literalCASCADE/ parameter is not given, error will be + thrown. + /para If schema is not the same. Here I think that you may directly refer to schema_name... - /* If the user is giving us the schema name, it must exist already */ + /* If the user is giving us the schema name, it must exist already. */ Noise? Intentional. Any back-patching there will be anyway complicated by the change of the code couple lines bellow and above so I think it's OK to fix the comment even if just cosmetically. With the patch: =# create extension adminpack schema popo2; ERROR: 3F000: schema popo2 does not exist LOCATION: get_namespace_oid, namespace.c:2873 On HEAD: =# create extension adminpack schema popo2; ERROR: 0A000: extension adminpack must be installed in schema pg_catalog LOCATION: CreateExtension, extension.c:1352 Time: 0.978 ms It looks like a regression to me to check for the existence of the schema before checking if the extension is compatible with the options given by users (regression test welcome). Yes that's what I meant by the change of checking order in the explanation above. I did that because I thought code would be more complicated otherwise, but apparently I was stupid... -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services From c13e2919466bb9001666a63361ff560d1779bde4 Mon Sep 17 00:00:00 2001 From: Petr Jelinek pjmodos@pjmodos.net Date: Sat, 13 Jun 2015 19:49:10 +0200 Subject: [PATCH] CREATE EXTENSION CASCADE --- contrib/hstore_plperl/expected/hstore_plperl.out | 6 +- contrib/hstore_plperl/expected/hstore_plperlu.out | 6 +- contrib/hstore_plperl/sql/hstore_plperl.sql| 4 +- contrib/hstore_plperl/sql/hstore_plperlu.sql | 4 +- .../hstore_plpython/expected/hstore_plpython.out | 4 +- contrib/hstore_plpython/sql/hstore_plpython.sql| 3 +- contrib/ltree_plpython/expected/ltree_plpython.out | 4 +- contrib/ltree_plpython/sql/ltree_plpython.sql | 3 +- doc/src/sgml/ref/create_extension.sgml | 42 + src/backend/commands/extension.c | 103 ++--- src/backend/parser/gram.y | 4 + src/bin/psql/tab-complete.c| 7 +- src/test/modules/Makefile | 1 + src/test/modules/test_extensions/.gitignore| 3 + src/test/modules/test_extensions/Makefile | 21 + .../test_extensions/expected/test_extensions.out | 30 ++ .../test_extensions/sql/test_extensions.sql| 18 .../modules/test_extensions/test_ext1--1.0.sql | 3 + src/test/modules/test_extensions/test_ext1.control | 5 + .../modules/test_extensions/test_ext2--1.0.sql | 3 + src/test/modules/test_extensions/test_ext2.control | 4 + .../modules/test_extensions/test_ext3--1.0.sql | 3 + src/test/modules/test_extensions/test_ext3.control | 3 + .../test_extensions/test_ext_cyclic1--1.0.sql | 3 + .../test_extensions/test_ext_cyclic1.control | 4 + .../test_extensions/test_ext_cyclic2--1.0.sql | 3 + .../test_extensions/test_ext_cyclic2.control | 4 + src/tools/msvc/Mkvcbuild.pm| 2 +- 28 files changed, 263 insertions(+), 37 deletions(-) create mode 100644 src/test/modules/test_extensions/.gitignore create mode 100644 src/test/modules/test_extensions/Makefile create mode 100644 src/test/modules/test_extensions/expected/test_extensions.out create mode 100644 src/test/modules/test_extensions/sql/test_extensions.sql create mode 100644 src/test/modules/test_extensions/test_ext1--1.0.sql create mode 100644 src/test/modules/test_extensions/test_ext1.control create mode 100644 src/test/modules/test_extensions/test_ext2--1.0.sql create mode 100644 src/test/modules/test_extensions/test_ext2.control create mode 100644
Re: [HACKERS] raw output from copy
2015-07-23 22:05 GMT+02:00 Dickson S. Guedes lis...@guedesoft.net: 2015-07-07 3:32 GMT-03:00 Pavel Stehule pavel.steh...@gmail.com: Hi previous patch was broken, and buggy Here is new version with fixed upload and more tests The interesting is so I should not to modify interface or client - so it should to work with any current driver with protocol support = 3. Hi Hi Pavel, Here are some thoughts: 1) from docs: only row data in network byte order are exported or imported. Should it be only raw data? I don't understand well - it use a PostgreSQL buildin send functions - and result of these functions is defined as in network byte order 2) from docs Because this format doesn't support any delimiter, only one value can be exported or imported. NULL values are not allowed. That only one value can be exported or imported is a little sad for someone with a table with more than one column that accepts bytea. The implemented feature doesn't covers the use-case where a table 'image' has columns: id integer, image bytea, thumbnail bytea, and I want to import binary data in that. We could put here the cases where we have NOT NULL columns. Since these are expected and the error messages complain about that couldn't them be covered in docs more explicitly? This mode should not to replace current COPY binary mode. RAW binary output for multiple fields is terrible complex task - you can use a fix length, you can use some special separator etc. I remember a terrible complex bulkload on Oracle or MSSQL - and I would to design it differently. I prefer to have a COPY statement simple as possible - If you need import/export all fields in record - then you can: 1. you can use a new LO api (for import) - load binary files as LO, INSERT and drop used LO 2. call more COPY statements, and join exported files with operation system tools (for export), 3. you can write specialized application that will support a COPY API and export, import data in your preferred format. The same complexity is with input, and I would not to write generic binary files parser. 3) from code: bool row_processed; /* true, when first row was processed */ in this mode is only one row - so first_row_processed sounds little bit strange. Maybe rename the variable to something like `first_row_processed` and rip off the comment? 4) from code: if (cstate-raw) format = 2; else if (cstate-binary) format = 1; else format = 0; Maybe create a constant for code readability? good idea If by one side this feature does not covers a more generalized case, by other is a nice start, IMHO. It is exactly what I don't would - the complexity of usage can go up to sky with generic binary format file processing. Regards Pavel -- Dickson S. Guedes mail/xmpp: gue...@guedesoft.net - skype: guediz http://github.com/guedes - http://guedesoft.net http://www.postgresql.org.br
Re: [HACKERS] multivariate statistics / patch v7
Hi, On 07/16/2015 01:51 PM, Kyotaro HORIGUCHI wrote: Hi, I'd like to show you the modified constitution of multivariate statistics application logic. Please find the attached. They apply on your v7 patch. Sadly I do have some trouble getting it to apply correctly :-( So for now all my comments are based on just reading the code. FWIW I've rebased my patch to the current master, it's available on github as usual: https://github.com/tvondra/postgres/commits/mvstats The code to find mv-applicable clause is moved out of the main flow of clauselist_selectivity. As I said in the previous mail, the new function transformRestrictInfoForEstimate (too bad name but just for PoC:) scans clauselist and generates RestrictStatsData struct which drives mv-aware selectivity calculation. This struct isolates MV and non-MV estimation. The struct RestrictStatData mainly consists of the following three parts, - clause to be estimated by current logic (MV is not applicable) - clause to be estimated by MV-staistics. - list of child RestrictStatDatas, which are to be run recursively. mvclause_selectivty() is the topmost function where mv stats works. This structure effectively prevents main estimation flow from being broken by modifying mvstats part. Although I haven't measured but I'm positive the code is far reduced from yours. I attached two patches to this message. The first one is to rebase v7 patch to current(maybe) master and the second applies the refactoring. I'm a little anxious about performance but I think this makes the process to apply mv-stats far clearer. Regtests for mvstats succeeded asis except for fdep, which is not implememted in this patch. What do you think about this? I'm not sure, at this point. I'm having a hard time understanding how exactly the code works - there are pretty much no comments explaining the implementation, so it takes time to understand the code. This is especially true about transformRestrictInfoForEstimate which is also quite long. I understand it's a PoC, but comments would really help. On a conceptual level, I think the idea to split the estimation into two phases - enrich the expression tree with nodes with details about stats etc, and then actually do the estimation in the second phase might be interesting. Not because it's somehow clearer, but because it gives us a chance to see the expression tree as a whole, with details about all the stats (with the current code we process/estimate the tree incrementally). But I don't really know how useful that would be. I don't think the proposed change makes the process somehow clearer. I know it's a PoC at this point, so I don't expect it to be perfect, but for me the original code is certainly clearer. Of course, I'm biased as I wrote the current code, and I (naturally) shaped it to match my ideas during the development process, and I'm much more familiar with it. Omitting the support for functional dependencies is a bit unfortunate, I think. Is that merely to make the PoC simpler, or is there something that makes it impossible to support that kind of stats? Another thing that I noticed is that you completely removed the code that combined multiple stats (and selected the best combination of stats). In other words, you've reverted to the intermediate single statistics approach, including removing the improved handling of OR clauses and conditions. It's a bit difficult to judge the proposed approach not knowing how well it supports those (quite crucial) features. What if it can't support some them., or what if it makes the code much more complicated (thus defeating the goal of making it more clear)? I share your concern about the performance impact - one thing is that this new code might be slower than the original one, but a more serious issue IMHO is that the performance impact will happen even for relations with no multivariate stats at all. The original patch was very careful about getting ~0% overhead in such cases, and if the new code does not allow that, I don't see this approach as acceptable. We must not put additional overhead on people not using multivariate stats. But I think it's worth exploring this idea a bit more - can you rebase it to the current patch version (as on github) and adding the missing pieces (functional dependencies, multi-statistics estimation and passing conditions)? One more thing - I noticed you extended the pg_operator catalog with a oprmvstat attribute, used to flag operators that are compatible with multivariate stats. I'm not happy with the current approach (using oprrest to do this decision), but I'm not really sure this is a good solution either. The culprit is that it only answers one of the two important questions - Is it compatible? How to perform the estimation? So we'd have to rely on oprrest anyway, when actually performing the estimation of a clause with compatible operator. And we'd have to keep in
Re: [HACKERS] PL/pgSQL, RAISE and error context
Hi
I am sending a next variant of filtering context patch.
postgres=# do $$ begin raise notice 'kuku'; end $$;
NOTICE: kuku
DO
Time: 2.441 ms
postgres=# do $$ begin raise exception 'kuku'; end $$;
ERROR: kuku
CONTEXT: PL/pgSQL function inline_code_block line 1 at RAISE
Time: 0.648 ms
postgres=# \set SHOW_CONTEXT always
postgres=# do $$ begin raise notice 'kuku'; end $$;
NOTICE: kuku
CONTEXT: PL/pgSQL function inline_code_block line 1 at RAISE
DO
Time: 0.702 ms
It is a variant, when I try to filter CONTEXT in libpq. There is little bit
less granularity on libpq side than server side, but still it is enough -
always, error, none.
This patch is without documentation, but basic regress tests works.
Regards
Pavel
2015-07-25 10:01 GMT+02:00 Pavel Stehule pavel.steh...@gmail.com:
2015-07-21 16:58 GMT+02:00 Merlin Moncure mmonc...@gmail.com:
On Tue, Jul 21, 2015 at 2:53 AM, Heikki Linnakangas hlinn...@iki.fi
wrote:
On 07/21/2015 10:38 AM, Pavel Stehule wrote:
where we are with this patch? Can I do some for it?
I still feel this approach is misguided, and we should be tweaking psql
and/or libpq instead. I don't feel strongly though, and if some other
committer wants to pick this up in its current form, I won't object. So
this
patch has reached an impasse, and if no-one else wants to pick this up,
I'm
going to mark this as Returned with Feedback and move on.
That's unfortunate. Maybe I'm missing something:
What does a client side implementation offer that a server side
implementation does not offer?
I have not any problem to change the filtering to client side. Primary
question is fix of PLpgSQL RAISE statement issue - The context field
filtering is a necessary follow-up and trivial in both cases.
In this case, it is acceptable for all?
Regards
Pavel
merlin
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
new file mode 100644
index 6181a61..7168809
*** a/src/bin/psql/command.c
--- b/src/bin/psql/command.c
*** SyncVariables(void)
*** 2029,2034
--- 2029,2035
/* send stuff to it, too */
PQsetErrorVerbosity(pset.db, pset.verbosity);
+ PQsetErrorContextVisibility(pset.db, pset.show_context);
}
/*
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
new file mode 100644
index d3e3114..0bc97de
*** a/src/bin/psql/help.c
--- b/src/bin/psql/help.c
*** helpVariables(unsigned short int pager)
*** 307,313
{
FILE *output;
! output = PageOutput(85, pager ? (pset.popt.topt) : NULL);
fprintf(output, _(List of specially treated variables.\n));
--- 307,313
{
FILE *output;
! output = PageOutput(86, pager ? (pset.popt.topt) : NULL);
fprintf(output, _(List of specially treated variables.\n));
*** helpVariables(unsigned short int pager)
*** 339,344
--- 339,345
fprintf(output, _( PROMPT2specify the prompt used when a statement continues from a previous line\n));
fprintf(output, _( PROMPT3specify the prompt used during COPY ... FROM STDIN\n));
fprintf(output, _( QUIET run quietly (same as -q option)\n));
+ fprintf(output, _( SHOW_CONTEXT when a error context will be displayed [always, error, none]\n));
fprintf(output, _( SINGLELINE end of line terminates SQL command mode (same as -S option)\n));
fprintf(output, _( SINGLESTEP single-step mode (same as -s option)\n));
fprintf(output, _( USER the currently connected database user\n));
diff --git a/src/bin/psql/settings.h b/src/bin/psql/settings.h
new file mode 100644
index d34dc28..5b49059
*** a/src/bin/psql/settings.h
--- b/src/bin/psql/settings.h
*** typedef struct _psqlSettings
*** 129,134
--- 129,135
const char *prompt2;
const char *prompt3;
PGVerbosity verbosity; /* current error verbosity level */
+ bool show_context;
} PsqlSettings;
extern PsqlSettings pset;
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
new file mode 100644
index 28ba75a..534c914
*** a/src/bin/psql/startup.c
--- b/src/bin/psql/startup.c
*** main(int argc, char *argv[])
*** 157,162
--- 157,163
SetVariable(pset.vars, PROMPT1, DEFAULT_PROMPT1);
SetVariable(pset.vars, PROMPT2, DEFAULT_PROMPT2);
SetVariable(pset.vars, PROMPT3, DEFAULT_PROMPT3);
+ SetVariable(pset.vars, SHOW_CONTEXT, error);;
parse_psql_options(argc, argv, options);
*** verbosity_hook(const char *newval)
*** 868,873
--- 869,895
PQsetErrorVerbosity(pset.db, pset.verbosity);
}
+ static void
+ show_context_hook(const char *newval)
+ {
+ if (newval == NULL)
+ pset.show_context = PQSHOW_CONTEXT_ERROR;
+ else if (pg_strcasecmp(newval, always) == 0)
+ pset.show_context = PQSHOW_CONTEXT_ALL;
+ else if (pg_strcasecmp(newval, error) == 0)
+ pset.show_context = PQSHOW_CONTEXT_ERROR;
+ else if (pg_strcasecmp(newval, none) == 0)
+
Re: [HACKERS] markup problems in row_security GUC docs
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 07/25/2015 03:26 PM, Kevin Grittner wrote: The use of the apostrophe character to quote allowed enumerated values seems wrong. Probably these should be replaced by literal tags. The reference to BYPASSRLS also seems like it deserves markup, and possibly an entry in the index. Does someone who is closer to the feature want to clean that up, or should I just do it? I'll take care of it. Joe -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.22 (GNU/Linux) iQIcBAEBAgAGBQJVtBjsAAoJEDfy90M199hluVYQAIEmSpSMuCIYRyzRDUTBdrsJ jgJQIJG/OByEz126SXw6g1+965vFk8feOGSsqji1Naugw5QfYOCF95BN4UJqa6aP T1pPQFR+Mrowyl121SzJiFIl4648sx5pIZ+bm1HNOqmkPAvZAJ2nXKKNankP4iBU aHeDXpqCuNdONZqygvwQ3rP49ai7GYkYC+1ae0B0JshxetbFu8yvuOVlNGFi5aZ9 CmTr60Z2mPby8Mqx6s4FmBhfP7W9c3KuYSTM6CZqGlhRx4hiUCUil0/XsQun7Z+/ dTFuhzO+nuO+78X73lCLbRgQ+d1f35LJQfAGr8ObmyHg4hgxp6LMpNVzyc/d6Ffg 9LufcrR6eA9FvRB2CtVJlq88HNUUR31/F6XbZtBk97hsYB54NB4kI4/8cR2kHBQV a+IekahAf4tbMK1wf5SIpNImvxAUKuduSEs9LJmSk5RuBsBink34YgmI6O11u7I2 btgQ6rWZ9lo+CV5bRn/vqtRCS06WNUEjjBM8jdntILxPIw/Pbc9OVkdb67kKBxFG 9mLPUQUPbdCi3wwGxmwvDA7f79G2fNI6QzGQP9Hzfdkz19hWvgz7R54ri9hh7hCI oCbRm/bqRLOtrTnh3r8+Z4W/mMplYdNCE9YY3T+pXHi9zMiKs9UaHGg2q/SyPzS6 7NfEgabP0bzVKdqTXoVn =l9GN -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] xlc 12.1 miscompiles 32-bit ginCompareItemPointers()
On Sun, Jul 19, 2015 at 04:32:16PM -0400, Noah Misch wrote:
On Sun, Jul 19, 2015 at 04:22:47PM -0400, Tom Lane wrote:
More: if the compiler does have a bug like that, how much confidence can
we have, really, that there are no other miscompiled places and won't be
any in the future? If we are going to support this configuration, I think
what the patch ought to do is disable PG_USE_INLINE globally when this
compiler is detected. That would revert the behavior to what it was
before 43d89a2.
That's a reasonable alternative.
Here's the patch implementing it.
diff --git a/config/test_quiet_include.h b/config/test_quiet_include.h
index f4fa4d3..732b231 100644
--- a/config/test_quiet_include.h
+++ b/config/test_quiet_include.h
@@ -7,3 +7,12 @@ fun()
{
return 0;
}
+
+/*
+ * IBM XL C/C++ for AIX, V12.1 miscompiles, for 32-bit, some inline
+ * expansions of ginCompareItemPointers() long long arithmetic. To take
+ * advantage of inlining, build a 64-bit PostgreSQL.
+ */
+#if defined(__ILP32__) defined(__IBMC__)
+#error known inlining bug
+#endif
--
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] Debugging buildfarm pg_upgrade check failures
On 07/25/2015 10:59 AM, Tom Lane wrote: Now that we've restored proper logging of make check, I looked into today's failure report from axolotl: http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=axolotldt=2015-07-24%2020%3A29%3A18 What evidently happened there is that pg_ctl start gave up waiting for the postmaster to start too soon. The postmaster log appears to contain LOG: database system was shut down at 2015-07-24 16:45:40 EDT FATAL: the database system is starting up LOG: MultiXact member wraparound protections are now enabled LOG: database system is ready to accept connections which indicates that it did successfully come up, but not till after one PQping probe from pg_ctl, which was rejected with still starting up. Meanwhile we've got this log output from pg_ctl: waiting for server to start stopped waiting pg_ctl: could not start server Examine the log output. Counting the dots indicates that pg_ctl gave up after precisely 5 seconds. Now, looking at the logic in pg_ctl's test_postmaster_connection(), the only explanation that seems to fit the observed output is that the stat() on the postmaster pidfile (at line 650 in HEAD) failed. It's not clear why though, since the postmaster was clearly still alive at this point, and we must have been able to read the pidfile earlier to construct a connection string, else there would have been no PQping attempt. Maybe the stat failed for some unexpected resource-exhaustion kind of reason? It seems plausible to me that we should change pg_ctl to only consider stat() failure to be a reason to give up waiting if errno is ENOENT, not anything else. At a minimum, I'd like to modify it to print the errno if it's anything else, so that we can confirm or deny this theory next time we see this buildfarm failure. Comments? Certainly let's look at the errno. cheers andrdew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] markup problems in row_security GUC docs
The use of the apostrophe character to quote allowed enumerated values seems wrong. Probably these should be replaced by literal tags. The reference to BYPASSRLS also seems like it deserves markup, and possibly an entry in the index. Does someone who is closer to the feature want to clean that up, or should I just do it? http://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=doc/src/sgml/config.sgml;h=b91d6c75d276e644583915485264b1787fb0c756;hb=master#l5561 -- Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: [HACKERS] A little RLS oversight?
On 07/22/2015 02:17 PM, Dean Rasheed wrote:
On 21 July 2015 at 04:53, Michael Paquier michael.paqu...@gmail.com wrote:
On Tue, Jul 14, 2015 at 4:01 AM, Stephen Frost sfr...@snowman.net wrote:
We need to be careful to avoid the slippery slope of trying to prevent
all covert channels, which has been extensively discussed previously.
I think this is more serious than the covert channel leaks discussed
before, since most_common_vals explicitly reveals values from the
table, making it an overt leak, albeit of a small portion of the
table's values.
Looking at that I am not seeing any straight-forward way to resolve
this issue except by hardening pg_stats by having an additional filter
of this type so as a non-owner of a relation cannot see the stats of
this table directly when RLS is enabled:
c.relrowsecurity = false OR c.relowner = current_user::regrole::oid
Attached is a patch doing that (/me now hides, expecting to receive
laser shots because of the use of current_user on a system view).
Thoughts?
Hmm, I think it probably ought to do more, based on whether or not RLS
is being bypassed or in force-mode -- see the first few checks in
get_row_security_policies(). Perhaps a new SQL-callable function
exposing those checks and calling check_enable_rls(). It's probably
still worth including the c.relrowsecurity = false check in SQL to
save calling the function for the majority of tables that don't have
RLS.
Please see the attached patch and let me know what you think. I believe
the only thing lacking is documentation for the two new user visible
functions. Comments?
Joe
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index e82a53a..c0bd6fa 100644
*** a/src/backend/catalog/system_views.sql
--- b/src/backend/catalog/system_views.sql
*** CREATE VIEW pg_indexes AS
*** 150,156
LEFT JOIN pg_tablespace T ON (T.oid = I.reltablespace)
WHERE C.relkind IN ('r', 'm') AND I.relkind = 'i';
! CREATE VIEW pg_stats AS
SELECT
nspname AS schemaname,
relname AS tablename,
--- 150,156
LEFT JOIN pg_tablespace T ON (T.oid = I.reltablespace)
WHERE C.relkind IN ('r', 'm') AND I.relkind = 'i';
! CREATE VIEW pg_stats WITH (security_barrier) AS
SELECT
nspname AS schemaname,
relname AS tablename,
*** CREATE VIEW pg_stats AS
*** 211,217
FROM pg_statistic s JOIN pg_class c ON (c.oid = s.starelid)
JOIN pg_attribute a ON (c.oid = attrelid AND attnum = s.staattnum)
LEFT JOIN pg_namespace n ON (n.oid = c.relnamespace)
! WHERE NOT attisdropped AND has_column_privilege(c.oid, a.attnum, 'select');
REVOKE ALL on pg_statistic FROM public;
--- 211,219
FROM pg_statistic s JOIN pg_class c ON (c.oid = s.starelid)
JOIN pg_attribute a ON (c.oid = attrelid AND attnum = s.staattnum)
LEFT JOIN pg_namespace n ON (n.oid = c.relnamespace)
! WHERE NOT attisdropped
! AND has_column_privilege(c.oid, a.attnum, 'select')
! AND (c.relrowsecurity = false OR NOT row_security_active(c.oid));
REVOKE ALL on pg_statistic FROM public;
diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c
index 3ca168b..094ac33 100644
*** a/src/backend/utils/adt/acl.c
--- b/src/backend/utils/adt/acl.c
*** static AclMode convert_priv_string(text
*** 92,98
static AclMode convert_any_priv_string(text *priv_type_text,
const priv_map *privileges);
- static Oid convert_table_name(text *tablename);
static AclMode convert_table_priv_string(text *priv_type_text);
static AclMode convert_sequence_priv_string(text *priv_type_text);
static AttrNumber convert_column_name(Oid tableoid, text *column);
--- 92,97
*** has_table_privilege_id_id(PG_FUNCTION_AR
*** 1998,2004
/*
* Given a table name expressed as a string, look it up and return Oid
*/
! static Oid
convert_table_name(text *tablename)
{
RangeVar *relrv;
--- 1997,2003
/*
* Given a table name expressed as a string, look it up and return Oid
*/
! Oid
convert_table_name(text *tablename)
{
RangeVar *relrv;
diff --git a/src/backend/utils/misc/rls.c b/src/backend/utils/misc/rls.c
index 44cb374..cd1a521 100644
*** a/src/backend/utils/misc/rls.c
--- b/src/backend/utils/misc/rls.c
***
*** 19,24
--- 19,25
#include catalog/pg_class.h
#include miscadmin.h
#include utils/acl.h
+ #include utils/builtins.h
#include utils/elog.h
#include utils/rls.h
#include utils/syscache.h
*** check_enable_rls(Oid relid, Oid checkAsU
*** 111,113
--- 112,141
/* RLS should be fully enabled for this relation. */
return RLS_ENABLED;
}
+
+ /*
+ * row_security_active variants
+ *
+ * check_enable_rls wrapped as a SQL callable function except
+ * RLS_NONE_ENV and RLS_NONE are the same for this purpose.
+ */
+ Datum
+
Re: [HACKERS] Anyone working on the TOAST items on the TODO list?
On 07/25/2015 07:05 AM, Bill Moran wrote: If not, I'm going to make a little personal project of them for myself (targeting 9.6). Nope. In fact, even the one which was 90% complete (replacing zlib with lz4) completely dropped off the radar. -- 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] pg_dump -Fd and compression level
Andrew Dunstan and...@dunslane.net writes: On 07/25/2015 01:52 PM, I wrote: I propose to tighten pg_dump's rules so that only 0..9 are accepted as arguments for -Z, and in compress_io.c:cfopen(), if compression is equal to Z_DEFAULT_COMPRESSION, not add any explicit compression value to the mode, thus using the zlib default. As per attached patch. Comments? Looks OK to me. Thanks for fixing 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] Anyone working on the TOAST items on the TODO list?
On Sat, 25 Jul 2015 11:39:15 -0700 Josh Berkus j...@agliodbs.com wrote: On 07/25/2015 07:05 AM, Bill Moran wrote: If not, I'm going to make a little personal project of them for myself (targeting 9.6). Nope. In fact, even the one which was 90% complete (replacing zlib with lz4) completely dropped off the radar. Interesting ... I wasn't looking at doing that, rather adjusting the decision logic on when to compress, as well as making the trigger values configurable so as to open the TOAST logic up to easy user configuration at the DB as well as the table level. I figure this will also need enough testing to feel like we're shipping with reasonable default values. I got the impression that this is something that's generally desired (it's something _I'd_ like to have). If I'm not misunderstanding, then I'll get started on a proposed patch. -- Bill Moran -- 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_dump -Fd and compression level
On 07/25/2015 01:52 PM, I wrote:
I propose to tighten pg_dump's rules so that only 0..9 are accepted as
arguments for -Z, and in compress_io.c:cfopen(), if compression is
equal to Z_DEFAULT_COMPRESSION, not add any explicit compression value
to the mode, thus using the zlib default.
As per attached patch.
Comments?
cheers
andrew
diff --git a/src/bin/pg_dump/compress_io.c b/src/bin/pg_dump/compress_io.c
index 912fc2f..6e1469b 100644
--- a/src/bin/pg_dump/compress_io.c
+++ b/src/bin/pg_dump/compress_io.c
@@ -547,11 +547,21 @@ cfopen(const char *path, const char *mode, int compression)
if (compression != 0)
{
#ifdef HAVE_LIBZ
- char mode_compression[32];
+ if (compression != Z_DEFAULT_COMPRESSION)
+ {
+ /* user has specified a compression level, so tell zlib to use it */
+ char mode_compression[32];
+
+ snprintf(mode_compression, sizeof(mode_compression), %s%d,
+ mode, compression);
+ fp-compressedfp = gzopen(path, mode_compression);
+ }
+ else
+ {
+ /* don't specify a level, just use the zlib default */
+ fp-compressedfp = gzopen(path, mode);
+ }
- snprintf(mode_compression, sizeof(mode_compression), %s%d,
- mode, compression);
- fp-compressedfp = gzopen(path, mode_compression);
fp-uncompressedfp = NULL;
if (fp-compressedfp == NULL)
{
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 0e036b8..b24b8bd 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -485,6 +485,11 @@ main(int argc, char **argv)
case 'Z': /* Compression Level */
compressLevel = atoi(optarg);
+if (compressLevel 0 || compressLevel 9)
+{
+ write_msg(NULL, compression level must be in range 0..9\n);
+ exit_nicely(1);
+}
break;
case 0:
--
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_dump -Fd and compression level
I propose to tighten pg_dump's rules so that only 0..9 are accepted as arguments for -Z, and in compress_io.c:cfopen(), if compression is equal to Z_DEFAULT_COMPRESSION, not add any explicit compression value to the mode, thus using the zlib default. As per attached patch. Comments? It seems that the first test on the compression in pg_backup_tar.c is now obsolete. It didn't make much sense anyway. 211 if (AH-compression 0 || AH-compression 9) 212 AH-compression = Z_DEFAULT_COMPRESSION; 213 214 /* Don't compress into tar files unless asked to do so */ 215 if (AH-compression == Z_DEFAULT_COMPRESSION) 216 AH-compression = 0; 217 218 /* 219 * We don't support compression because reading the files back is not 220 * possible since gzdopen uses buffered IO which totally screws file 221 * positioning. 222 */ 223 if (AH-compression != 0) 224 exit_horribly(modulename, 225 compression is not supported by tar archive format\n); 226 } regards, Marc 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] Anyone working on the TOAST items on the TODO list?
Bill Moran wmo...@potentialtech.com writes: On Sat, 25 Jul 2015 11:39:15 -0700 Josh Berkus j...@agliodbs.com wrote: Nope. In fact, even the one which was 90% complete (replacing zlib with lz4) completely dropped off the radar. Interesting ... I wasn't looking at doing that, rather adjusting the decision logic on when to compress, as well as making the trigger values configurable so as to open the TOAST logic up to easy user configuration at the DB as well as the table level. I figure this will also need enough testing to feel like we're shipping with reasonable default values. I have a vague feeling that we've accreted some code that has dependencies on the current TOAST behavior. Unfortunately it's no more than a vague feeling so I can't tell you where to look; but it'd be a good idea to look around and/or test, rather than just assume we can let users frob these knobs to whatever random settings they feel like. I don't want to discourage you from the idea of making these things accessible, just suggesting that there may be more work to do than simply making them accessible. There may be a need for fairly strict sanity checking on the values. 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] polymorphic types - enforce casting to most common type automatically
2015-07-22 10:37 GMT+02:00 Heikki Linnakangas hlinn...@iki.fi:
On 07/11/2015 12:19 AM, Pavel Stehule wrote:
2015-07-10 18:43 GMT+02:00 Tom Lane t...@sss.pgh.pa.us:
An example of what would presumably happen if we adopted this sort of
rule
(I've not checked whether the patch as written does this, but it would
logically follow) is that appending a float to an integer array would
cause the whole array to be silently promoted to float, with attendant
possible loss of precision for existing array elements.
it is based on select_common_type() - so it is use only available implicit
casts.
Without patch:
postgres=# select pg_typeof(array_append('{1,2,3}'::int[], 1.2::float));
ERROR: function array_append(integer[], double precision) does not exist
With patch:
postgres=# select pg_typeof(array_append('{1,2,3}'::int[], 1.2::float));
pg_typeof
double precision[]
(1 row)
Yeah, I agree with Tom that we don't want that change in behaviour. I'll
mark this as rejected.
ok.
I accept it - it introduce incompatible changes.
Still I am sure, so this behave is some what users expect, but it should
not be introduced this way.
Any ideas how this issues can be solved?
Regards
Pavel
- Heikki
Re: [HACKERS] extend pgbench expressions with functions
Hello Heikki,
As an initial list of functions, I'd suggest:
abs(x)
min(x, y, ...)
max(x, y, ...)
random_uniform(min, max)
random_gaussian(min, max, threshold)
random_exponential(min, max, threshold)
Would that be enough to specify e.g. the
As soon as we add more functions, the way they are documented needs to be
reworked too; we'll need to add a table in the manual to list them.
Here is a v8 with abs, min, max, random, gaussian et
exponential.
There is no expression typing, but expressions are evaluated and cast to
the expected type depending on what is needed (variables expect an int
value, function arguments are usually int but for threshold...).
There is an evalDouble function to evaluate double arguments for
gaussian exponential thresholds.
There is a special int function which evaluates its argument as a double
and cast the result to an int, for testing purpose.
Variables are only int values, and functions only return ints.
There is no real doc, WIP...
Also included are a set of sql stuff I used for minimal tests.
--
Fabien.diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index 2517a3a..ed99abc 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -758,10 +758,11 @@ pgbench optional replaceableoptions/ /optional replaceabledbname/
Sets variable replaceablevarname/ to an integer value calculated
from replaceableexpression/.
The expression may contain integer constants such as literal5432/,
- references to variables literal:/replaceablevariablename/,
+ double constants such as literal3.14156/,
+ references to integer variables literal:/replaceablevariablename/,
and expressions composed of unary (literal-/) or binary operators
(literal+/, literal-/, literal*/, literal//, literal%/)
- with their usual associativity, and parentheses.
+ with their usual associativity, integer function calls and parentheses.
/para
para
diff --git a/src/bin/pgbench/exprparse.y b/src/bin/pgbench/exprparse.y
index e68631e..a1b0afd 100644
--- a/src/bin/pgbench/exprparse.y
+++ b/src/bin/pgbench/exprparse.y
@@ -16,10 +16,14 @@
PgBenchExpr *expr_parse_result;
+static PgBenchExprList *make_elist(PgBenchExpr *exp, PgBenchExprList *list);
static PgBenchExpr *make_integer_constant(int64 ival);
+static PgBenchExpr *make_double_constant(double dval);
static PgBenchExpr *make_variable(char *varname);
static PgBenchExpr *make_op(char operator, PgBenchExpr *lexpr,
PgBenchExpr *rexpr);
+static int find_func(const char * fname);
+static PgBenchExpr *make_func(const int fnumber, PgBenchExprList *args);
%}
@@ -29,15 +33,19 @@ static PgBenchExpr *make_op(char operator, PgBenchExpr *lexpr,
%union
{
int64 ival;
+ double dval;
char *str;
PgBenchExpr *expr;
+ PgBenchExprList *elist;
}
+%type elist elist
%type expr expr
-%type ival INTEGER
-%type str VARIABLE
+%type ival INTEGER function
+%type dval DOUBLE
+%type str VARIABLE FUNCTION
-%token INTEGER VARIABLE
+%token INTEGER DOUBLE VARIABLE FUNCTION
%token CHAR_ERROR /* never used, will raise a syntax error */
/* Precedence: lowest to highest */
@@ -49,6 +57,10 @@ static PgBenchExpr *make_op(char operator, PgBenchExpr *lexpr,
result: expr{ expr_parse_result = $1; }
+elist: expr { $$ = make_elist($1, NULL); }
+ | elist ',' expr { $$ = make_elist($3, $1); }
+ ;
+
expr: '(' expr ')' { $$ = $2; }
| '+' expr %prec UMINUS { $$ = $2; }
| '-' expr %prec UMINUS { $$ = make_op('-', make_integer_constant(0), $2); }
@@ -58,7 +70,12 @@ expr: '(' expr ')' { $$ = $2; }
| expr '/' expr { $$ = make_op('/', $1, $3); }
| expr '%' expr { $$ = make_op('%', $1, $3); }
| INTEGER{ $$ = make_integer_constant($1); }
+ | DOUBLE{ $$ = make_double_constant($1); }
| VARIABLE { $$ = make_variable($1); }
+ | function '(' elist ')'{ $$ = make_func($1, $3); }
+ ;
+
+function: FUNCTION { $$ = find_func($1); pg_free($1); }
;
%%
@@ -74,6 +91,16 @@ make_integer_constant(int64 ival)
}
static PgBenchExpr *
+make_double_constant(double dval)
+{
+ PgBenchExpr *expr = pg_malloc(sizeof(PgBenchExpr));
+
+ expr-etype = ENODE_DOUBLE_CONSTANT;
+ expr-u.double_constant.dval = dval;
+ return expr;
+}
+
+static PgBenchExpr *
make_variable(char *varname)
{
PgBenchExpr *expr = pg_malloc(sizeof(PgBenchExpr));
@@ -95,4 +122,89 @@ make_op(char operator, PgBenchExpr *lexpr, PgBenchExpr *rexpr)
return expr;
}
+static struct {
+ char * fname;
+ int nargs;
+ PgBenchFunction tag;
+} PGBENCH_FUNCTIONS[] = {
+ { abs, 1, PGBENCH_ABS },
+ { int, 1, PGBENCH_INT },
+ { min, -1, PGBENCH_MIN },
+ { max, -1, PGBENCH_MAX },
+ { random, 2, PGBENCH_RANDOM },
+ { gaussian, 3, PGBENCH_GAUSSIAN },
+ { exponential, 3, PGBENCH_EXPONENTIAL }
+};
+
+static int
+find_func(const char * fname)
+{
+ int nfunctions = sizeof(PGBENCH_FUNCTIONS) / sizeof(PGBENCH_FUNCTIONS[0]);
+ int i;
+
+ for (i = 0; i nfunctions; i++)
+ if
Re: [HACKERS] Anyone working on the TOAST items on the TODO list?
On Sat, 25 Jul 2015 16:06:28 -0400 Tom Lane t...@sss.pgh.pa.us wrote: Bill Moran wmo...@potentialtech.com writes: On Sat, 25 Jul 2015 11:39:15 -0700 Josh Berkus j...@agliodbs.com wrote: Nope. In fact, even the one which was 90% complete (replacing zlib with lz4) completely dropped off the radar. Interesting ... I wasn't looking at doing that, rather adjusting the decision logic on when to compress, as well as making the trigger values configurable so as to open the TOAST logic up to easy user configuration at the DB as well as the table level. I figure this will also need enough testing to feel like we're shipping with reasonable default values. I have a vague feeling that we've accreted some code that has dependencies on the current TOAST behavior. Unfortunately it's no more than a vague feeling so I can't tell you where to look; but it'd be a good idea to look around and/or test, rather than just assume we can let users frob these knobs to whatever random settings they feel like. I don't want to discourage you from the idea of making these things accessible, just suggesting that there may be more work to do than simply making them accessible. There may be a need for fairly strict sanity checking on the values. Good to know. I guess it's a good thing that I'm giving myself a full year to work on this ... -- Bill Moran -- 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_dump -Fd and compression level
On 07/25/2015 03:07 PM, Marc Mamin wrote: I propose to tighten pg_dump's rules so that only 0..9 are accepted as arguments for -Z, and in compress_io.c:cfopen(), if compression is equal to Z_DEFAULT_COMPRESSION, not add any explicit compression value to the mode, thus using the zlib default. As per attached patch. Comments? It seems that the first test on the compression in pg_backup_tar.c is now obsolete. It didn't make much sense anyway. 211 if (AH-compression 0 || AH-compression 9) 212 AH-compression = Z_DEFAULT_COMPRESSION; 213 214 /* Don't compress into tar files unless asked to do so */ 215 if (AH-compression == Z_DEFAULT_COMPRESSION) 216 AH-compression = 0; 217 218 /* 219 * We don't support compression because reading the files back is not 220 * possible since gzdopen uses buffered IO which totally screws file 221 * positioning. 222 */ 223 if (AH-compression != 0) 224 exit_horribly(modulename, 225 compression is not supported by tar archive format\n); 226 } In fact, the first two tests look unnecessary. Neither condition should be possible now. 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] markup problems in row_security GUC docs
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 07/25/2015 04:17 PM, Joe Conway wrote: On 07/25/2015 03:26 PM, Kevin Grittner wrote: The use of the apostrophe character to quote allowed enumerated values seems wrong. Probably these should be replaced by literal tags. The reference to BYPASSRLS also seems like it deserves markup, and possibly an entry in the index. Does someone who is closer to the feature want to clean that up, or should I just do it? I'll take care of it. I pushed a fix for this. There are no current index entries for BYPASSRLS, so none added. Arguably there could be more improvements with respect to index entries for RLS, but I'll save that for another time. - -- Joe Conway -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.22 (GNU/Linux) iQIcBAEBAgAGBQJVtC6bAAoJEDfy90M199hlow4QAJuQePEwM1t+3P8V+Qwt0a5P sWL8i0UcEWIa6a/+s3SYZ+8j0QbXNdkruNNx+dYKrXMk5ZiicCfbvO/9FJeBRtW7 IjmXw6ootfg13uw4Rql4iOZLArSd7D1Lrb3T5NOTSkNktrNKLOfEPNrxps73H0rR V5/TXfIcgLtWDvVA7k8RsUJt5uln/dw5n7JvpesncZqEEH2OQpSDOQZNJLxf83h+ gR0u6EFaWM7j9oQD0vVucqKCj3U+y93jwk5ZMEOeADUDw+ceM3FY/Pss5yLyIh7g pySejFxmQavIuIKYapsACZwXg2Ro5Fs9QYErCwP3BBIk1277baMUYO+/NRnrwcFg /VxehrkWC+JvFRlvVHeiIntwtKve0dhpcDxgl9ukbQ9mnYMmh4DoObjsSrusXLXQ TX1XlCgI44QCqJ98qtje7r9cTUDVV0JgDkXJDeQ9YiWInsEXVkU560ioe+Ke09Fa eof3X1FcR32TGFp7hEpSDs0EHZewd1eGae+4f3RdcANw0Gq14mmIs5sbxEaq9ip9 7+KP85O4jTAHIb6KgBbu80dAfyENjAh7v08UZixte1C+9rJ7wcXah63NxQV6QM24 6kWsVtPpImf4ZmSv4vF+Xi1wTOPkChGMzthz4lpwTt43ZdGE9xgBLJ3fwDGV/Vil HaQyH6gX05LPuQ+usW86 =d3qv -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
[HACKERS] [DESIGN] ParallelAppend
Hello, I'm recently working/investigating on ParallelAppend feature towards the next commit fest. Below is my design proposal. 1. Concept -- Its concept is quite simple anybody might consider more than once. ParallelAppend node kicks background worker process to execute child nodes in parallel / asynchronous. It intends to improve the performance to scan a large partitioned tables from standpoint of entire throughput, however, latency of the first multi-hundred rows are not scope of this project. From standpoint of technology trend, it primarily tries to utilize multi-cores capability within a system, but also enables to expand distributed database environment using foreign-tables inheritance features. Its behavior is very similar to Funnel node except for several points, thus, we can reuse its infrastructure we have had long- standing discussion through the v9.5 development cycle. 2. Problems to be solved - Typical OLAP workloads takes tons of tables join and scan on large tables which are often partitioned, and its KPI is query response time but very small number of sessions are active simultaneously. So, we are required to run a single query as rapid as possible even if it consumes larger computing resources than typical OLTP workloads. Current implementation to scan heap is painful when we look at its behavior from the standpoint - how many rows we can read within a certain time, because of synchronous manner. In the worst case, when SeqScan node tries to fetch the next tuple, heap_getnext() looks up a block on shared buffer, then ReadBuffer() calls storage manager to read the target block from the filesystem if not on the buffer. Next, operating system makes the caller process slept until required i/o get completed. Most of the cases are helped in earlier stage than the above worst case, however, the best scenario we can expect is: the next tuple already appear on top of the message queue (of course visibility checks are already done also) with no fall down to buffer manager or deeper. If we can run multiple scans in parallel / asynchronous, CPU core shall be assigned to another process by operating system, thus, it eventually improves the i/o density and enables higher processing throughput. Append node is an ideal point to be parallelized because - child nodes can have physically different location by tablespace, so further tuning is possible according to the system landscape. - it can control whether subplan is actually executed on background worker, per subplan basis. If subplan contains large tables and small tables, ParallelAppend may kick background worker to scan large tables only, but scan on small tables are by itself. - Like as Funnel node, we don't need to care about enhancement of individual node types. SeqScan, IndexScan, ForeignScan or others can perform as usual, but actually in parallel. 3. Implementation -- * Plan Cost ParallelAppend shall appear where Appen can appear except for the usage for dummy. So, I'll enhance set_append_rel_pathlist() to add both of AppendPath and ParallelAppendPath with cost for each. Cost estimation logic shall take further discussions, however, I expect the logic below to estimate the cost for ParallelAppend. 1. Sum startup_cost and run_cost for each child pathnode, but distinguish according to synchronous or asynchronous. Probably, total cost of pathnode is less than: (parallel_setup_cost + its total cost / parallel_append_degree + number of rows * cpu_tuple_comm_cost) is nonsense to run on background worker. 2. parallel_setup_cost * (# of asynchronous nodes) are added to sum of startup_cost of asynchronous nodes. 3. sum of run_cost of asynchronous nodes are divided by parallel_append_degree, then cpu_tuple_comm_cost * (total # of rows by asynchronous nodes) are added. 4. both of synchronous and asynchronous cost are added, then it becomes the cost of ParallelAppend. Obviously, it stand on the viewpoint that says: cost reflects response time of the underlying plan. So, cost of ParallelAppend can be smaller than sum of underlying child nodes. * Execution Like Funnel node, it kicks background worker on the ExecProcNode handler, thus, its startup time may be later than Fujita-san's approach if call of ParallelAppend would be late. For example, when ParallelAppend is located under HashJoin but inner Hash loads billion of rows. Even though I expect ExecParallelAppend takes, at least, simple round- robin scheduling like funnel_getnext(), we may give synchronous nodes than asynchronous just after the background worker startup. 4. Further challenges -- * Serialization of CustomScan via outfuncs.c/readfuncs.c Because methods field is, basically, a set of pointers per process basis, we need to have an infrastructure to reproduce same table on the background worker process identified by the
Re: [HACKERS] RFC: replace pg_stat_activity.waiting with something more descriptive
On Fri, Jul 24, 2015 at 1:12 PM, Ildus Kurbangaliev
i.kurbangal...@postgrespro.ru wrote:
On Jul 24, 2015, at 7:26 AM, Amit Kapila amit.kapil...@gmail.com
wrote:
2.
+const char *
+pgstat_get_wait_event_name(uint8 classId, uint8 eventId)
{
..
}
I don't understand why a single WaitEventType enum is not sufficient
as in the patch provided by me and why we need to divide it into
separate structures for each kind of wait, I find that way easily
understandable.
In current patch if somebody adds new lock or individual lwlock he just
add
its name to the corresponding array. With WaitEventType first of all he
suppose to
know about WaitEventType,
That anyway he has to do it either you go by defining individual arrays
or having unified WaitEventType enum for individual arrays he has to
find out that array. Another thing is with that you can just encapsulate
this information in one byte in structure PgBackendStatus, rather than
using more number of bytes (4 bytes) and I think the function for reporting
Waitevent will be much more simplified.
I think it is better if we just implement the idea of tranche's on top of my
patch and do the other remaining work like setting of bwaiting correctly
as mentioned upthread.
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
Re: [HACKERS] [PATCH] postgres_fdw extension support
On Sat, Jul 25, 2015 at 2:19 AM, Paul Ramsey pram...@cleverelephant.ca wrote: On Thu, Jul 23, 2015 at 7:48 AM, Paul Ramsey pram...@cleverelephant.ca wrote: Here's an updated patch that clears the cache on changes to foreign wrappers and servers. Any chance one of you folks could by my official commitfest reviewer? Appreciate all the feedback so far! https://commitfest.postgresql.org/6/304/ That's an interesting topic, I will register as such. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] creating extension including dependencies
On Sat, Jul 25, 2015 at 12:59 AM, Petr Jelinek p...@2ndquadrant.com wrote: On 2015-07-22 07:12, Michael Paquier wrote: On Tue, Jul 21, 2015 at 11:20 PM, Tom Lane t...@sss.pgh.pa.us wrote: Petr Jelinek p...@2ndquadrant.com writes: ... My main question is if we are ok with SCHEMA having different behavior with CASCADE vs without CASCADE. I went originally with no and added the DEFAULT flag to SCHEMA. If the answer is yes then we don't need the flag (in that case CASCADE acts as the flag). Yeah, I was coming around to that position as well. Insisting that SCHEMA throw an error if the extension isn't relocatable makes sense as long as only one extension is being considered, but once you say CASCADE it seems like mostly a usability fail. I think it's probably OK if with CASCADE, SCHEMA is just use if needed else ignore. Here is a patch implementing that. Note that the checks are now done in different order for non-relocatable extension when SCHEMA is specified than previously. Before the patch, the SCHEMA was first checked for conflict with the extension's schema and then there was check for schema existence. This patch always checks for schema existence first, mainly to keep code saner (to my eyes). +In case the extension specifies schema in its control file, the schema +can't be overriden using literalSCHEMA/ parameter. The actual +behavior of the literalSCHEMA/ parameter in this case will depend +on circumstances: SCHEMA is not a parameter, it is a clause. Same for CASCADE. Also schema here should use the markup structfield as it is referenced as the parameter of a control file. + para + If schema is not same as the one in extension's control file and + the literalCASCADE/ parameter is not given, error will be + thrown. + /para If schema is not the same. Here I think that you may directly refer to schema_name... - /* If the user is giving us the schema name, it must exist already */ + /* If the user is giving us the schema name, it must exist already. */ Noise? +# test_ddl_deparse must be first +REGRESS = test_extensions Why is test_ddl_deparse mentioned here? With the patch: =# create extension adminpack schema popo2; ERROR: 3F000: schema popo2 does not exist LOCATION: get_namespace_oid, namespace.c:2873 On HEAD: =# create extension adminpack schema popo2; ERROR: 0A000: extension adminpack must be installed in schema pg_catalog LOCATION: CreateExtension, extension.c:1352 Time: 0.978 ms It looks like a regression to me to check for the existence of the schema before checking if the extension is compatible with the options given by users (regression test welcome). -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Supporting TAP tests with MSVC and Windows
On Sat, Jul 25, 2015 at 2:27 AM, Heikki Linnakangas hlinn...@iki.fi wrote: On 06/25/2015 07:40 AM, Michael Paquier wrote: On Tue, May 26, 2015 at 3:39 PM, Michael Paquier wrote: Here is v6, a rebased version on HEAD (79f2b5d). There were some conflicts with the indentation and some other patches related to pg_rewind and initdb's tests. Attached is v7, rebased on 0b157a0. Thanks! I fiddled with this a bit more, to centralize more of the platform-dependent stuff to RewindTest.pm. Also, Windows doesn't have cat and touch if you haven't installed MinGW, so I replaced those calls with built-in perl code. Can you double-check that the attached still works in your environment? It works for me now. Sure, I'll double check and update your last version as necessary. We need to put some instructions in the docs on how to install IPC::Run on Windows. I can write up something unless you're eager to. I'll do it. That's fine, but it is going to be closer to a ninja version of what you did. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_dump -Fd and compression level
On 07/25/2015 03:20 AM, Andrew Dunstan wrote: On 07/25/2015 02:34 AM, Marc Mamin wrote: Andrew Dunstan and...@dunslane.net writes: Hmm. Yeah. It looks like commit a7ad5cf0cfcfab8418000d652fa4f0c6ad6c8911 changed from using the default compression for libz to using the compression set in pg_dump options, which defaults to 0. This actually seems like the right thing to do, but it certainly should have been called out much more forcefully in release notes, and arguably should not have been changed in stable releases. Not sure what we do about it now. really 0? wouldn't that mean no compression at all? No, that's not right either. The default should be Z_DEFAULT_COMPRESSION, so we shouldn't actually see a difference in the default case. And it is definitely compressing some. So I'm now puzzled by what you're seeing. OK, I have got this worked out. I'll have a bug fix shortly. 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_dump -Fd and compression level
On Sat, Jul 25, 2015 at 9:56 PM, Andrew Dunstan and...@dunslane.net wrote: On 07/25/2015 03:20 AM, Andrew Dunstan wrote: On 07/25/2015 02:34 AM, Marc Mamin wrote: Andrew Dunstan and...@dunslane.net writes: Hmm. Yeah. It looks like commit a7ad5cf0cfcfab8418000d652fa4f0c6ad6c8911 changed from using the default compression for libz to using the compression set in pg_dump options, which defaults to 0. This actually seems like the right thing to do, but it certainly should have been called out much more forcefully in release notes, and arguably should not have been changed in stable releases. Not sure what we do about it now. really 0? wouldn't that mean no compression at all? No, that's not right either. The default should be Z_DEFAULT_COMPRESSION, so we shouldn't actually see a difference in the default case. And it is definitely compressing some. So I'm now puzzled by what you're seeing. OK, I have got this worked out. I'll have a bug fix shortly. So you are basically planning to switch to Z_BEST_SPEED instead of Z_DEFAULT_COMPRESSION where needed for the default code path? -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
