Re: [COMMITTERS] pgsql: Remove pgbench "progress" test pending solution of its timing is
Hello Tom,
Well, I think it's mostly about valgrind making everything really slow.
Since we have seen some passes from skink recently, perhaps there was
also a component of more-load-on-the-machine-than-usual. But in the end
this is just evidence for my point that regression tests have to be very
much not timing-sensitive. We run them under all kinds of
out-of-the-ordinary stress.
Attached is an attempt at improving the situation:
(1) there are added comments to explain the whys, which is to provide
coverage for pgbench time-related features... while still not being
time-sensitive, which is a challenge.
(2) the test now only expects "progress: \d" from the output, so it is
enough that one progress is shown, whenever it is shown.
(3) if the test is detected to have gone AWOL, detailed log checks are
coldly skipped.
This would have passed on "skink" under the special conditions it
encountered.
I cannot guaranty that it would pass under any circumstance, though.
If it still encounters a failure, ISTM that it should only be a missing
"progress:" in the output, which has not been encountered so far.
If it occurs, a few options would remain, none of them very convincing:
- give the test some more time, eg 3 seconds (hmmm... could still fail
after any time...)
- drop the "progress:" expectation (hmmm... but then it does not check
anything).
- make the "progress:" output check conditional to the running time
(hmmm... it would require changing the command_checks_all function,
and there is still a chance that the bench was stuck for 2 seconds
then given time to realize that it has to stop right now...)
- use an even simpler transaction, eg "SELECT;" which is less likely to
get stuck (but still could get stuck...).
For the random-ness related test (--sample-rate), we could add a feature
to pgbench which allows to control the random seed, so that the number of
samples could be known in advance for testing purposes.
--
Fabien.diff --git a/src/bin/pgbench/t/001_pgbench_with_server.pl b/src/bin/pgbench/t/001_pgbench_with_server.pl
index 11bc0fe..5043504 100644
--- a/src/bin/pgbench/t/001_pgbench_with_server.pl
+++ b/src/bin/pgbench/t/001_pgbench_with_server.pl
@@ -4,13 +4,14 @@ use warnings;
use PostgresNode;
use TestLib;
use Test::More;
+use Time::HiRes qw{time};
# start a pgbench specific server
my $node = get_new_node('main');
$node->init;
$node->start;
-# invoke pgbench
+# invoke pgbench, return elapsed time
sub pgbench
{
my ($opts, $stat, $out, $err, $name, $files) = @_;
@@ -32,10 +33,13 @@ sub pgbench
append_to_file($filename, $$files{$fn});
}
}
+
+ my $t0 = time();
$node->command_checks_all(\@cmd, $stat, $out, $err, $name);
# cleanup?
#unlink @filenames or die "cannot unlink files (@filenames): $!";
+ return time() - $t0;
}
# Test concurrent insertion into table with UNIQUE oid column. DDL expects
@@ -445,14 +449,53 @@ sub check_pgbench_logs
ok(unlink(@logs), "remove log files");
}
-# with sampling rate
+# note: --progress-timestamp is not tested
+
+# The point of this test is coverage of various time-related features
+# (-T, -P, --aggregate-interval, --rate, --latency-limit...), so it is
+# somehow time sensitive.
+# The checks performed are quite loose so as to still pass even under
+# degraded (high load, slow host, valgrind run) testing conditions.
+# Maybe it might fail if no every second progress report is
+# shown over 2 seconds...
+my $elapsed = pgbench(
+ '-T 2 -P 1 -l --log-prefix=001_pgbench_log_1 --aggregate-interval=1'
+ . ' -S -b se@2 --rate=20 --latency-limit=1000 -j ' . $nthreads
+ . ' -c 3 -r',
+ 0,
+ [ qr{type: multiple},
+ qr{clients: 3},
+ qr{threads: $nthreads},
+ # the shown duration is really -T argument value
+ qr{duration: 2 s},
+ qr{script 1: .* select only},
+ qr{script 2: .* select only},
+ qr{statement latencies in milliseconds},
+ qr{FROM pgbench_accounts} ],
+ [ qr{vacuum},
+ # hopefully at least expect some progress report?
+ qr{progress: \d\b} ],
+ 'pgbench progress');
+
+# if the test has gone AWOL, coldly skip these detailed checks.
+if (abs($elapsed - 2.0) < 0.5)
+{
+ # $nthreads threads, 2 seconds, but due to timing imprecision we might get
+ # only 1 or as many as 3 progress reports per thread.
+ check_pgbench_logs('001_pgbench_log_1', $nthreads, 1, 3,
+ qr{^\d+ \d{1,2} \d+ \d+ \d+ \d+ \d+ \d+ \d+ \d+ \d+$});
+}
+
+# with sampling rate, not time sensitive
pgbench(
'-n -S -t 50 -c 2 --log --log-prefix=001_pgbench_log_2 --sampling-rate=0.5',
0,
[ qr{select only}, qr{processed: 100/100} ],
- [qr{^$}],
+ [ qr{^$} ],
'pgbench logs');
+# random 50% of 2*50 transactions, accept between 8 and 92
+# probability of failure is about 2 * 2^-42 (?)
check_pgbench_logs('001_pgbench_log_2', 1, 8, 92,
qr{^0 \d{1,2} \d+ \d \d+ \d+$});
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postg
Re: [COMMITTERS] pgsql: Remove pgbench "progress" test pending solution of its timing is
You are attacking a straw man. I thought it was tilting at windmills:-) In the TAP-test context, you do have quite a bit of freedom to decide what is a pass and what is a fail. So maybe the path forward is to be more flexible about the pass conditions for this test. Hmmm. I'm not sure that there is a solution under "any" testing conditions if -T & -P are to be tested and some sensible output expected out of them. I'll think about it. But I can't help thinking that these results have exposed some outright bugs too. Yep, I agree that there is probably one bug/race/unexpected condition wrt what is output and time out handling. -- Fabien. -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Remove pgbench "progress" test pending solution of its timing is
If the test must run even when postgres doesn't, it is a little bit hard as a starting assumption for a benchmarking tool:-( I'm unsure what the point of this is. It's not like we discussing removing pgbench, we're talking about an unreliable test. My sentence was probably not very clear. I just meant that devising a coverage test which does not fail when the benchmarking tool is not able to run because of load/valgrind/... is kind of hard. Anything even loosely related to time, here having a few simple SELECT transactions over 2 seconds and a few printf into a file or stdout every second, can always fail if conditions are bad enough. So this means somehow giving up on coverage, because of one host which can fail under very unusual testing conditions once in a while. I would prefer to keep the test and have a warning instead, something like "ok, although a test which is allowed to rarely fail failed", but at least the feature are tested most of the time. Sigh. -- Fabien. -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Remove pgbench "progress" test pending solution of its timing is
Hello Tom, Remove pgbench "progress" test pending solution of its timing issues. It would have been nicer to simply comment them out... Buildfarm member skink shows that this is even more flaky than I thought. There are probably some actual pgbench bugs here as well as a timing dependency. According to the trace, the host was so loaded that it could not do anything for over two seconds, so that the first output is "progress: 2.6", i.e. the 0 thread did not get any significant time for the first 2.6 seconds... of a 1 second test. Hmmm, not very kind. Somehow this cannot be helped: if the system does not give any execution time to some pgbench thread, the expected output cannot be there. The checked condition could be relaxed to "progress: \d", which still assumes that the system was kind enough to give enough cpu time to the process in the two second run so that it could output something. The issue is probably the same on the second test on the generated log. My guess is that some transactions (we are talking about "-S"...) took several seconds, thus were completed and aggregate in rows over the expected limit. Could be considered bug, that is the aggregation should discard over the limit results rather than counting them in some late entry. I'll investigate that. Globally these test failures reveals pathological testing conditions: the inability to execute a simple select over several seconds. It could have been OOM or whatever else. If the test must run even when postgres doesn't, it is a little bit hard as a starting assumption for a benchmarking tool:-( -- Fabien. -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Provide a test for variable existence in psql
Provide a test for variable existence in psql Thanks for the editing & commit. Pavel's feedback helped to devise a nicer syntax, although it was not strictly speaking a review. -- Fabien. -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Fix more portability issues in new pgbench TAP tests.
Fix more portability issues in new pgbench TAP tests. Sorry for all these issues unforeseen issues. Thanks to the farm! * Remove no-such-user test case, output isn't stable, and we really don't need to be testing such cases here anyway. Ok. * Fix the process exit code test logic to match PostgresNode::psql (but I didn't bother with looking at the "core" flag). Ok. At least it is consistent. * Give up on inf/nan tests. Too bad, I liked them! Thanks for the fixes. -- Fabien. -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Fix assorted portability issues in new pgbench TAP tests.
Hello,
Please find attached "blind" additional fixes for Windows & AIX.
- more nan/inf variants
- different message on non existing user
- illegal vs unrecognized options
I suspect that $windows_os is not true on "bowerbird", in order to fix it
the value of "$Config{osname}" is needed...
--
Fabien.diff --git a/src/bin/pgbench/t/001_pgbench_with_server.pl b/src/bin/pgbench/t/001_pgbench_with_server.pl
index 66df4bc..54a6039 100644
--- a/src/bin/pgbench/t/001_pgbench_with_server.pl
+++ b/src/bin/pgbench/t/001_pgbench_with_server.pl
@@ -73,7 +73,11 @@ pgbench(
1,
[qr{^$}],
[ qr{connection to database "template0" failed},
- qr{FATAL: role "no-such-user" does not exist} ],
+ # FATAL: role "no-such-user" does not exist
+ # FATAL: SSPI authentication failed for user "no-such-user"
+ qr{FATAL:.* (role|user) "no-such-user"},
+ qr{FATAL:.* (does not exist|authentication failed)}
+ ],
'no such user');
pgbench(
@@ -217,9 +221,9 @@ pgbench(
qr{command=18.: double 18\b},
qr{command=19.: double 19\b},
qr{command=20.: double 20\b},
- qr{command=21.: double -?nan}i,
- qr{command=22.: double inf}i,
- qr{command=23.: double -inf}i,
+ qr{command=21.: double (-?nan|-1\.#IND)}i,
+ qr{command=22.: double (inf|1\.#INF)}i,
+ qr{command=23.: double (-inf|-1\.#INF)}i,
qr{command=24.: int 9223372036854775807\b}, ],
'pgbench expressions',
{ '001_pgbench_expressions' => q{-- integer functions
diff --git a/src/bin/pgbench/t/002_pgbench_no_server.pl b/src/bin/pgbench/t/002_pgbench_no_server.pl
index 631aa73..d6b3d4f 100644
--- a/src/bin/pgbench/t/002_pgbench_no_server.pl
+++ b/src/bin/pgbench/t/002_pgbench_no_server.pl
@@ -26,7 +26,7 @@ my @options = (
# name, options, stderr checks
[ 'bad option',
'-h home -p 5432 -U calvin -d --bad-option',
- [ qr{unrecognized option}, qr{--help.*more information} ] ],
+ [ qr{(unrecognized|illegal) option}, qr{--help.*more information} ] ],
[ 'no file',
'-f no-such-file',
[qr{could not open file "no-such-file":}] ],
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Fix assorted portability issues in new pgbench TAP tests.
Fix assorted portability issues in new pgbench TAP tests. Most where hard to guess without having the report. Thanks. * Test was way too optimistic about the platform independence of NaN and Infinity outputs. I rather imagine we might have to lose those tests altogether, but for the moment just allow case variation and fully spelled out Infinity. Yep. I've seen strange things on these. I wonder whether all test platform are IEEE 754 conforming. -- Fabien. -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [HACKERS] [COMMITTERS] pgsql: Add missing support for new node fields
Hello Andres, It is not done yet, but it looks that it can work in the end with limited effort. Currently it works for copy & equal. It'd have to do out/read as well imo. Sure. This part is WIP, though. Is there some interest to generate the x00kB of sources rather than edit them everytime, or forgetting it from time to time, or does everyone like it as it is? From my POV yes. But it's not quite as trivial as just generating it based on fields. Some fields are intentionally skipped, e.g. location, for equalfuncs, but not copy/out/readfuncs. So there needs to be a way to specify such special rules. Indeed, I noticed these particularities... For some case it can be automated with limited input. For really special cases (eg a few read/out functions) , the script is told to skip some node types and the special manual version is taken from the file instead of being generated. -- Fabien. -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Add missing support for new node fields
Add missing support for new node fields Commit b6fb534f added two new node fields but neglected to add copy and comparison support for them, Mea culpa, should have checked for that. I've been annoyed by these stupid functions and forgetting to update them since I run into them while trying to fix an issue in pg_stat_statement some time ago. I've started to develop a perl script to generate most of them from headers. It is not done yet, but it looks that it can work in the end with limited effort. Currently it works for copy & equal. Is there some interest to generate the x00kB of sources rather than edit them everytime, or forgetting it from time to time, or does everyone like it as it is? -- Fabien. -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Fix pgbench's failure to honor the documented long-form option "
Not only did it not accept --builtin as a synonym for -b, but what it did accept as a synonym was --tpc-b (huh?), which it got even further wrong by marking as no_argument, so that if you did try that you got a core dump. I suppose this is leftover from some early design for the new switches added by commit 8bea3d221, but it's still pretty sloppy work. Argh, shame on me. I think at some point --tpc-b was an alias for the default script which then got replaced by builtin + name, but the implementation did not follow the upgrade:-( Sorry for the result. -- Fabien. -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Refactor script execution state machine in pgbench.
Will fix. Oops, I missed this line... Thanks! -- Fabien. -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Refactor script execution state machine in pgbench.
Hello Tom,
I think the problem is here (pgbench.c lines 4550):
Hmmm... Indeed "typedef char bool;". I thought postgresql bool was an
int, I usually do "typedef enum { false, true } bool;"
I see a number of other things that look pretty infelicitous in this
code --- for example, why is the loop at lines 4440 break'ing after
it comes across the first socket to wait for?
Indeed this seems strange. I will have a look at that as well.
I'll try to look at these tonight, or if I can't by the end of the week.
--
Fabien.
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Refactor script execution state machine in pgbench.
Hello Tom, There's at least one bug in this patch. Indeed. It's caused pgbench to go into an infinite loop during the TAP tests on my buildfarm machine longfin. buildfarm 719:14.18 /Users/buildfarm/bf-data/HEAD/inst/bin/pgbench --no-vacuum --client=5 --protocol=prepared --transactions=25 --file /Users/buildfarm/bf-data/HEAD/pgsql.build/src/bin/pgbench/tmp_check/data_main_JNh2/pgbench_script Some other buildfarm critters seem to have got through that successfully, so it's probably a portability issue not a hard failure. Hmmm, not easy to identify, especially as I do not have a clang on MacOS... Could you catch the process to identify the infinite loop? What are the states of the five clients? I think that it may be stuck in doCustom or on threadRun. I'm not sure how it could be stuck in doCustom. Re-reading the code, I think that the "end_tx_processed" stuff could be removed in favor of a systematic return at the end of the transaction, but that does not explain anything wrt to an infinite loop: after 25 transactions it should have got to FINISHED and have exited. Maybe it can be stuck in threadRun in the "while (remains>0)" loop, which suggests that a remains-- has been forgotten... but I do not see how this is possible to forget a client by reading the code. So although I hastily declared that bugs would be easier to detect, without any additional clue it is hard to help. -- Fabien. -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Rename pgbench min/max to least/greatest, and fix handling of do
Rename pgbench min/max to least/greatest, and fix handling of double args. Ok. The very minor patch attached re-establishes the alphabetical order when listing functions names in pgbench documentation. -- Fabien.diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml index e6c7c94..ee363ef 100644 --- a/doc/src/sgml/ref/pgbench.sgml +++ b/doc/src/sgml/ref/pgbench.sgml @@ -955,13 +955,6 @@ pgbench options dbname 5432.0 - int(x) - integer - cast to int - int(5.4 + 3.8) - 9 - - greatest(a [, ... ] ) double if any a is double, else integer largest value among arguments @@ -969,6 +962,13 @@ pgbench options dbname 5 + int(x) + integer + cast to int + int(5.4 + 3.8) + 9 + + least(a [, ... ] ) double if any a is double, else integer smallest value among arguments -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: doc: Fix typos
Hello Peter, http://git.postgresql.org/pg/commitdiff/b87b2f4bda1a3b98f8dea867b8bc419ace7a9ea9 doc/src/sgml/ref/pgbench.sgml | 8 - Here is example outputs: + Here is example output: I think that something is missing now on this line. Should it rather be "Here is an example output", if singular is better? It looks awkward to me without some article. -- Fabien. -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: pgbench: Allow changing weights for scripts
ISTM that in stack variables are initialized to zero automatically, so although it is not explicitely initialized, it is not uninitialized... Uh? They're not. Indeed, I mixed up with "static", shame on me! -- Fabien. -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: pgbench: Allow changing weights for scripts
pgbench.c:2765: warning: 'ps.stats.lag.sum2' is used uninitialized in this function Sorry for the noise. Strangely, I did not get that warning with gcc 4.8.4. ISTM that in stack variables are initialized to zero automatically, so although it is not explicitely initialized, it is not uninitialized... Moreover, these values are not actually "used" in the function, they are just returned, and are to be initialized explicitely later on. So gcc is wrong, but that does not mean that it should not be fixed. Alvaro has just done that, which is overall a code improvement, so it is for the better, and it shows that a wrong warning can lead to actual benefits:-) -- Fabien. -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Fix bug slowing down pgbench when -P is used.
A removed check in ba3deeefb Indeed. It seems that it was really removed by 1bc90f7a, that I should have checked... -- Fabien. -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: pgbench: Add a real expression syntax to \set
Also, it appears that the exprscan.h file doesn't actually get (or need to be) generated here. Indeed. So I think we need something like the attached. There is a $(RM) make macro usually defined as "rm -f", but it does not seem to be used elsewhere. -- Fabien. -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: pgbench: Add a real expression syntax to \set
The makefile changes in this do not look right to me. Files that are meant to be shipped in the tarball should be removed by make maintainer-clean, not an ordinary "make clean" which is what the committed patch appears to do. Otherwise, a tarball user without bison installed would be cut off at the knees if he did "make clean" or "make distclean"; but those operations are not supposed to remove files that were in the tarball. Ooops, sorry, I'm probably responsible for the wrong EXTRA_CLEAN. I definitely did not consider that clean was expected to leave the directory to be compilable without flex/bison. -- Fabien. -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Support more commands in event triggers
Hello Alvaro, I have submitted a small patch to improve tests on event triggers, including tests for GRANT/REVOKE/COMMENT: https://commitfest.postgresql.org/5/179/ -- Fabien. -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Support more commands in event triggers
doc/src/sgml/event-trigger.sgml | 125 +- While reading the documentation, it occured to me that the first sentence of the first subsection lacks a reference to "table_rewrite", see attached very minor suggestion. BTW, I'll try to send a "check" script. -- Fabien.diff --git a/doc/src/sgml/event-trigger.sgml b/doc/src/sgml/event-trigger.sgml index 04353ea..48ac1ac 100644 --- a/doc/src/sgml/event-trigger.sgml +++ b/doc/src/sgml/event-trigger.sgml @@ -30,7 +30,8 @@ supported events are ddl_command_start, ddl_command_end - and sql_drop. + sql_drop + and table_rewrite. Support for additional events may be added in future releases. -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Add: > * Add tool to query pg_stat_* tables
Hello, * Add tool to query pg_stat_* tables and report indexes that aren't needed or tables that might need indexes Good. This is the kind of things I'm planning for the "pg-advisor" tool, which queries system relations thru views and report inconsistencies (such as mismatching types for foreign references...). This particular one is not checked, but could be added quite easily. See http://pgfoundry.org/projects/pg-advisor/ for details and a prototype. I must admit I don't have much time recently so the project is "stuck", although not forgotten. -- Fabien. ---(end of broadcast)--- TIP 8: explain analyze is your friend
Re: [COMMITTERS] pgsql-server: Fix unintended assignment of sequences
Dear Tom, > Fix unintended assignment of sequences to the containing schema's > default tablespace You might consider fixing the "CREATE SCHEMA" documentation as well. Thanks, have a nice day, -- Fabien Coelho - [EMAIL PROTECTED] ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [COMMITTERS] pgsql-server: > Please find enclose a submission
Dear Tom, > I am not any keener than you on this particular approach ... the nesting > of ifdefs in the makefiles seems mighty ugly. I agree with you. > But I'd like to find a way to clean it up, not just claim that we don't > need to do it. At least six suggestions, make your choice: (1) submitted with the first version: having a separate makefile for pgxs, and use "make -f the-pgxs-makefile target" -> the pgxs makefile is very similar to the contrib one. -> it looked a burden to have two sets of makefiles to be maintained. -> thus Peter suggested that it was not a good idea, and I agreed. (2) having the ifdef stuff in the common part included by contrib makefile, contrib/contrib-global.mk. done in submission 3 I think. -> it was breaking the "vpath" feature, because of a chicken-and-egg problem on the definition of top_srcdir, which required two separate includes, although one include is fine with pgxs. -> thus Peter rejected it. I agree that existing things must not be broken. (3) the ifdef stuff is put in every makefile. done in submission 4. -> this is the current status in dev. -> it solves both previous issues. -> Tom finds it ugly, and he is right;-) (4) remove in-source tree compilation, so that there is only pgxs. -> that would make much simpler makefiles -> that would break the vpath feature. -> that would mean that compiling/installing contrib must be done AFTER the server is compiled and installed. From an administrator point of view, I don't see that as a big issue. That's the case with apache external modules and I have never found it a problem in the past years. -> However ISTM that it is a political decision that pg-managers are not akin to make. (5) improve pgxs so that it works easilly both in-source and out-source. But I don't know how to that cleanly and without the kind of ifdef that are already there and are found ugly... Maybe solution (2) could be improved so that it works fine for vpath? As I'm not a vpath user, I'm not sure whether it can be achieved. Peter would be the best man to look into that, but the previous version he proposed broke pgxs because it required to include Makefile.global which is not there under pgxs. (6) provide the same functionnality with any other mean: a special executable like apache "apxs" command, or whatever. I did not chose this approach because it looked quite simple to adapt the existing contrib infrastructure to make it work outside of the source tree... indeed, all the work was already done and it was more a re-packaging issue. If someone else want to do it, fine with me, I don't stick to my particular implementation: I just want the feature;-) Have a nice day, -- Fabien Coelho - [EMAIL PROTECTED] ---(end of broadcast)--- TIP 8: explain analyze is your friend
Re: [COMMITTERS] pgsql-server: Remove NT4 mention in release notes.
Dear Bruce, > Remove NT4 mention in release notes. please find attached a patch to make it even clearer that NT4 is not supported, by *not* using "NT-based Windows" expression as NT4 is not included, and by specify explicitely NT4 as not supported... Have a nice day, -- Fabien Coelho - [EMAIL PROTECTED]*** ./doc/src/sgml/release.sgml.origWed Aug 18 18:22:44 2004 --- ./doc/src/sgml/release.sgml Wed Aug 18 18:26:53 2004 *** *** 29,36 This is the first PostgreSQL release to natively run on Microsoft Windows as a server. It can run as a Windows service. This release ! supports NT-based Windows releases like Win2000, XP, Win2003. ! Older releases like Windows 95, 98, and ME are not supported because these operating systems do not have the infrastructure to support PostgreSQL. A separate installer project has been created to ease installation on Windows: --- 29,36 This is the first PostgreSQL release to natively run on Microsoft Windows as a server. It can run as a Windows service. This release ! supports Windows releases Win2000, XP and Win2003. ! Older releases like Windows 95, 98, NT4 and ME are not supported because these operating systems do not have the infrastructure to support PostgreSQL. A separate installer project has been created to ease installation on Windows: ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [COMMITTERS] pgsql-server: PostgreSQL extension makefile framework
Dear Peter,
> PostgreSQL extension makefile framework ("pgxs"), by Fabien Coelho, with
> some massaging by Peter Eisentraut. This is basically a simple
> generalization of the existing contrib makefiles.
Thanks for your help.
I'm having a look at CVS know, and it seems to me that one cannot pgxs
"dynamic" stuff to install any contrib, because:
(1) all makefiles in contrib include directly "src/Makefile.global" which
is generated by configure, although it is already included by the
"src/makefiles/pgxs.mk" makefile anyway, so it seems to me that it
is useless because redundant?
(2) only static includes are used, so I cannot use "pg_config --pgxs"
approach to install a contrib against an already compiled postgresql?
basically, you drop the USE_PGXS stuff for dynamic configuration
of contrib makefiles, and I have found no replacement.
That does annoy me a little bit not to be able to use it, as it was one of
my motivations for doing all that stuff...
May I enquire on the rational for the current status?
Would it be possible to have both worlds?
If not, what is the actual issue?
What was broken with version 4 I sent?
Or am I missing something somewhere? I'm pretty jetlagged this afternoon...:-(
Thanks in advance,
--
Fabien Coelho - [EMAIL PROTECTED]
---(end of broadcast)---
TIP 8: explain analyze is your friend
Re: [COMMITTERS] pgsql-server/src backend/utils/adt/acl.c inclu ...
I agree with you that work is needed in the acl area. > I hope you understand the larger scale of problems that would be created > if your patch got accepted. I would be ready to bet that I would be the only user for these functions. So large looks like me alone;-) Have a nice day, -- Fabien Coelho - [EMAIL PROTECTED] ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [COMMITTERS] pgsql-server/src backend/utils/adt/acl.c inclu ...
> OK, patch reverted at request of Tom and Peter. Please propose a more > generalitzed soluion. Thanks. Sigh. You refuse a 10 lines patch to access a stupid opaque type in the backend. You both refuse some things to be done in the back end, and also to add what is needed to do it in userland. Moreover, I don't think this patch did hurt anybody, as it was pretty invisible and was useful to me. I'm tired. When I'll be too tired, you'll just lose a contributor. A very small loss indeed, but I don't think it is a good policy for your project. Have a nice day, -- Fabien Coelho - [EMAIL PROTECTED] ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faqs/FAQ.html
Re: [COMMITTERS] pgsql-server/src backend/utils/adt/acl.c inclu ...
Dear Tom, > http://developer.postgresql.org/docs/postgres/functions-misc.html > Arguably these functions do not belong right there, but that's hardly a > reason to think that they do not need documentation. Sure. I was planing to add something anyway. > Personally, though, I think that Peter's original objection was right > on. We shouldn't be exporting these functions at all; it is right to > treat aclitem as an opaque type. I disagree strongly on this point. As I stated previously, it should a general principle that all information about internal configuration should be available from SQL, and better if in a relationnal form. This is among the "10 rules" of what a relationnal DB should do, as far as I can remember. Otherwise, it means that you do not trust SQL and the relationnal DB as a general tool. As a leading developper in a RDB system, I cannot believe that;-) > The problem with allowing computations on aclitems to occur in > client-side code I'm developping some "pg_advisor" stuff to check for many things in the database. YOU decided that all that should not be on the server side. Fine, I agree. Now if you want to keep things opaque in the server... > is that we will be locking ourselves into the present representation of > access rights, which is pretty durn foolish. I perfectly agree with you on this point;-) The "pg_hba.conf" code is pretty disappointing. I mean by that low level, no real internal data structure The aclitem stuff violates all rules I teach to my student about sound design: it is a array (no NF1) the same field references keys in different arrays, a null array means something implicitly, aso. > considering that we *know* we need to make changes in that area pretty > soon to move closer to SQL compliance (the whole users/groups/roles > business). The correct approach is not to export low-level access and > put functionality in the client, but to put the functionality on the > server side where it's convenient to change it at the same time we > reimplement ACLs. Well, it would be no big stuff to adapt this. > Ergo, my recommendation is to revert this change altogether. You're the boss. > Fabien should figure out the high-level description of what he wants to > know (at a level similar to has_table_privilege() and its ilk) and > propose server-side functions to implement that. Sure, I did that already. I built a plpgsql functions to return appropriate relations that I can query. However this plpgsql needs to access your "opaque" type. I can load the functions, but they seem to me that they belong to the backend. "has_*_privileges()" is NOT relationnal as it hides queries, so it does not really suit queries that want to deal with all possible users/groups and all possible objects. Moreover, I need access to the raw information to check for its consistency, not the derived functionnal stuff. -- Fabien Coelho - [EMAIL PROTECTED] ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faqs/FAQ.html
Re: [COMMITTERS] pgsql-server/src backend/utils/adt/acl.c inclu ...
> > Please find a attached a small patch that adds accessor functions > > for "aclitem" so that it is not an opaque datatype. > > Shouldn't this patch include some documentation updates? Well I thought about that, but I wasn't sure were to put it. There is no documentation at all about the aclitem data type. I'll submit a few lines anyway. -- Fabien Coelho - [EMAIL PROTECTED] ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
