Re: [HACKERS] pgbench: faster version of tpcb-like transaction
About the patch: I'm generally in favor of providing more options to pgbench, especially if it can give optimization ideas to the performance conscious user. I think that the name should be "tpcb-like-plfunc": the script does not implement tpcb per spec, and such a function could be written in another language with some performance benefit, or not. Maybe that mean to relax the prefix condition to "take the first matching name" when prefix are used. If you are reimplementing the transaction anyway, you could consider using UPDATE RETURNING instead of SELECT to get the balance. On the other hand the doc says that the "steps" are put in a PL function, so maybe it should reflect the original script. I'm surprised by: "select * from pgbench_transaction(:aid, :bid, :tid, :delta);\n" Why not simply: "select pgbench_transaction(:aid, :bid, :tid, :delta);\n" I would suggest to use a more precise function name, in case other functions are thought of. Maybe "pgbench_tpcb_like_plfunc". I would suggest to indent better the PL/function and put keywords and types in capital, and add explicitely the properties of the function (eg STRICT, VOLATILE?). There is a spurious space at the end of the executeStatement call line. The patch potentially interacts with other patches in the long and slow queue... As usual with pgbench there are no regression tests. -- 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] More replication race conditions
On Sun, Aug 27, 2017 at 12:03 PM, Tom Lane wrote: > And *another* replication test race condition just now: > > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=dangomushi&dt=2017-08-26%2019%3A37%3A08 > > As best I can interpret this, it's pointing out that this bit in > src/test/recovery/t/009_twophase.pl: > > $cur_master->psql( > 'postgres', " > BEGIN; > CREATE TABLE t_009_tbl2 (id int, msg text); > SAVEPOINT s1; > INSERT INTO t_009_tbl2 VALUES (27, 'issued to ${cur_master_name}'); > PREPARE TRANSACTION 'xact_009_13'; > -- checkpoint will issue XLOG_STANDBY_LOCK that can conflict with lock > -- held by 'create table' statement > CHECKPOINT; > COMMIT PREPARED 'xact_009_13';"); > > $cur_standby->psql( > 'postgres', > "SELECT count(*) FROM t_009_tbl2", > stdout => \$psql_out); > is($psql_out, '1', "Replay prepared transaction with DDL"); > > contains exactly no means of ensuring that the master's transaction has > been replayed on the standby before we check for its results. It's not > real clear why it seems to work 99.99% of the time, because, well, there > isn't any frickin' interlock there ... I have noticed this one this morning, and I am planning to address it with a proper patch soonishly. (I am still fighting a bit to get dangomushi in a more stable stable, and things run slow on it, so it is good at catching race conditions of this kind). -- 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] Variable substitution in psql backtick expansion
Spending developer time to write code for the hypothetical someone running a psql version 11 linked to a libpq < 7.4, if it can even link, does not look like a very good investment... Anyway, here is required the update. The question is the server's version, not libpq. Ok. Modern psql does still talk to ancient servers (I tried 11devel against 7.2 just now, to be sure). Wow:-) The stuff in describe.c may not work well, but basic functionality is there and I don't want to break it. Ok. Then the updated patch should work, although I do not have a setup to test that easily. -- 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] pgbench: faster version of tpcb-like transaction
Hello Tom, I dunno, it seems like this proposal involves jacking up the test case and driving a completely different one underneath. There is no reason to consider that you've improved the benchmark results --- you've just substituted a different benchmark, one with no historical basis, and not a lot of field justification either. ISTM that putting some SQL in a function and calling it is standard practice in some classes of applications, although probably not the most frequent. Moreover, as far as the TPC-B benchmark is concerned, it looks like a perflectly legitimate implementation of the benchmark: the transaction profile (Section 1.2) is described as 4 inputs sent in and one result returned. The fact that the SQL commands are sent one at a time by the client to the server is a pgbench choice that I would not have done if I wanted to show the greatest TPC-B numbers with Pg. Nor does it mean that it is a bad idea to do so... For instance an ORM web application might tend to generate simple unprepared CRUD queries and interact a lot back and forth, and the default test is not to bad to reflect that particular kind of behavior. Basically there are different kind of application implementations and different tests could reflect those. So I am fine with providing more benchmarking options to pgbench, and I intend to do so once its capabilities are improved. A caveat I have with Jeff patch is that "tpcb-func" is a misnommer because pgbench does NOT implement tpcb per spec, and it is my intention to propose a variant which does implement the spec when possible. Now I think that I'm also responsible for the prefix constraint on names... -- 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] pgbench: faster version of tpcb-like transaction
Hello, If all the data is in memory and you have a system with fast fsyncs (or are running with fsync off, or unlogged tables, or synchronous_commit off), then the big bottleneck in pgbench is the amount of back and forth between the pgbench program and the backend. Sure. The throughput of a benchmark depends on a bottleneck which may be disk ios, cpu, network, load... depending on the test conditions. I tested quite a few variants for my PgDay Paris 2017 talk, including PL functions, see https://wiki.postgresql.org/wiki/PgDay_Paris_2017. -- 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] pgbench: faster version of tpcb-like transaction
Jeff Janes writes: > If all the data is in memory and you have a system with fast fsyncs (or are > running with fsync off, or unlogged tables, or synchronous_commit off), > then the big bottleneck in pgbench is the amount of back and forth between > the pgbench program and the backend. There are 7 commands per transaction. Yeah ... > It is easy to package 5 of those commands into a single PL/pgSQL function, > with the other two being implicit via the standard auto-commit behavior > when explicit transactions are not opened. The attached patch does that, > under the name tpcb-func. I first named it tpcb-like-func, but one builtin > name can't be a prefix or another so that won't work. I dunno, it seems like this proposal involves jacking up the test case and driving a completely different one underneath. There is no reason to consider that you've improved the benchmark results --- you've just substituted a different benchmark, one with no historical basis, and not a lot of field justification either. > Wanting to measure IPC overhead is a valid thing to do, but > certainly isn't the most common thing people want to do with pgbench. I think that's nonsense. Measuring how fast PG can do client interactions is EXACTLY what this is about. Certainly, pushing SQL operations into server-side functions is a great way to reduce network overhead, but it has nothing to do with what we choose as a benchmark. 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] More replication race conditions
And *another* replication test race condition just now: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=dangomushi&dt=2017-08-26%2019%3A37%3A08 As best I can interpret this, it's pointing out that this bit in src/test/recovery/t/009_twophase.pl: $cur_master->psql( 'postgres', " BEGIN; CREATE TABLE t_009_tbl2 (id int, msg text); SAVEPOINT s1; INSERT INTO t_009_tbl2 VALUES (27, 'issued to ${cur_master_name}'); PREPARE TRANSACTION 'xact_009_13'; -- checkpoint will issue XLOG_STANDBY_LOCK that can conflict with lock -- held by 'create table' statement CHECKPOINT; COMMIT PREPARED 'xact_009_13';"); $cur_standby->psql( 'postgres', "SELECT count(*) FROM t_009_tbl2", stdout => \$psql_out); is($psql_out, '1', "Replay prepared transaction with DDL"); contains exactly no means of ensuring that the master's transaction has been replayed on the standby before we check for its results. It's not real clear why it seems to work 99.99% of the time, because, well, there isn't any frickin' interlock there ... 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] More replication race conditions
On Fri, Aug 25, 2017 at 12:09:00PM +0200, Petr Jelinek wrote: > On 24/08/17 19:54, Tom Lane wrote: > > sungazer just failed with > > > > pg_recvlogical exited with code '256', stdout '' and stderr > > 'pg_recvlogical: could not send replication command "START_REPLICATION SLOT > > "test_slot" LOGICAL 0/0 ("include-xids" '0', "skip-empty-xacts" '1')": > > ERROR: replication slot "test_slot" is active for PID 8913148 > > pg_recvlogical: disconnected > > ' at > > /home/nm/farm/gcc64/HEAD/pgsql.build/src/test/recovery/../../../src/test/perl/PostgresNode.pm > > line 1657. > > > > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=sungazer&dt=2017-08-24%2015%3A16%3A10 > > > > Looks like we're still not there on preventing replication startup > > race conditions. > > Hmm, that looks like "by design" behavior. Slot acquiring will throw > error if the slot is already used by somebody else (slots use their own > locking mechanism that does not wait). In this case it seems the > walsender which was using slot for previous previous step didn't finish > releasing the slot by the time we start new command. We can work around > this by changing the test to wait perhaps. [Action required within three days. This is a generic notification.] The above-described topic is currently a PostgreSQL 10 open item. Simon, since you committed the patch believed to have created it, you own this open item. If some other commit is more relevant or if this does not belong as a v10 open item, please let us know. Otherwise, please observe the policy on open item ownership[1] and send a status update within three calendar days of this message. Include a date for your subsequent status update. Testers may discover new open items at any time, and I want to plan to get them all fixed well in advance of shipping v10. Consequently, I will appreciate your efforts toward speedy resolution. Thanks. [1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: Poor cost estimate with interaction between table correlation and partial indexes
Do you think this is a reasonable approach? Should I start working on a patch based on the solution I described or is there some other approach I should look into? -- 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: faster version of tpcb-like transaction
On Sat, Aug 26, 2017 at 4:59 PM, Jeff Janes wrote: > I still get a 2 fold improvement, from 13668 to 27036, when both > transactions are tested with -M prepared. > > I am surprised, I usually haven't seen that much difference for the default > queries between prepared or not, to the point that I got out of the habit of > testing with it. But back when I was testing with and without > systematically, I did notice that it changed a lot depending on hardware and > concurrency. And of course from version to version different bottlenecks > come and go. I must admit that I had a similar unpleasant surprise at one point -- "-M prepared" seems to matter *a lot* these days. That's the default that I'd change, if any. -- Peter Geoghegan -- 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: faster version of tpcb-like transaction
On Sat, Aug 26, 2017 at 4:28 PM, Peter Geoghegan wrote: > On Sat, Aug 26, 2017 at 3:53 PM, Jeff Janes wrote: > > I get nearly a 3 fold speed up using the new transaction, from 9184 to > 26383 > > TPS, on 8 CPU machine using scale 50 and: > > > > PGOPTIONS="-c synchronous_commit=off" pgbench -c32 -j32 -T60 -b tpcb-like > > What about with "-M prepared"? I think that most of us use that > setting already, especially with CPU-bound workloads. > I still get a 2 fold improvement, from 13668 to 27036, when both transactions are tested with -M prepared. I am surprised, I usually haven't seen that much difference for the default queries between prepared or not, to the point that I got out of the habit of testing with it. But back when I was testing with and without systematically, I did notice that it changed a lot depending on hardware and concurrency. And of course from version to version different bottlenecks come and go. And thanks to Tom for letting me put -M at the end of the command line now. Cheers, Jeff
Re: [HACKERS] pgbench: faster version of tpcb-like transaction
On Sat, Aug 26, 2017 at 3:53 PM, Jeff Janes wrote: > I get nearly a 3 fold speed up using the new transaction, from 9184 to 26383 > TPS, on 8 CPU machine using scale 50 and: > > PGOPTIONS="-c synchronous_commit=off" pgbench -c32 -j32 -T60 -b tpcb-like What about with "-M prepared"? I think that most of us use that setting already, especially with CPU-bound workloads. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Poor cost estimate with interaction between table correlation and partial indexes
Hi. I'm looking to get started contributing code to Postgres. A small issue I'm aware of that I think would make a good first contribution is a poor cost estimate made due to an interaction between table correlation and partial indexes. Currently the planner assumes that when an index is perfectly correlated with a table and a range scan is performed on the index, all of the table page reads performed by the index scan except for the first one will be sequential reads. While this assumption is correct for regular indexes, it is not true for partial indexes. The assumption holds for regular indexes because the rows corresponding to two entries in a regular index that is perfectly correlated with the table are guaranteed to be next to each other in the table. On the other hand with a partial index perfectly correlated with a table, there may be rows in the table in between the two rows corresponding to two adjacent entries in the index that are not included in the index because they do not satisfy the partial index predicate. To make the cost calculation for this case more accurate, I want to apply the same estimate as the one currently used to estimate the cost of a bitmap heap scan. The bitmap heap scan cost estimate applies in this case because both cases involve reading pages from disk ordered by the location in the table, but where the pages may not be consecutive. For the relevant functions, see cost_index and cost_index_heap_scan in costsize.c. Thanks, Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] pgbench: faster version of tpcb-like transaction
If all the data is in memory and you have a system with fast fsyncs (or are running with fsync off, or unlogged tables, or synchronous_commit off), then the big bottleneck in pgbench is the amount of back and forth between the pgbench program and the backend. There are 7 commands per transaction. It is easy to package 5 of those commands into a single PL/pgSQL function, with the other two being implicit via the standard auto-commit behavior when explicit transactions are not opened. The attached patch does that, under the name tpcb-func. I first named it tpcb-like-func, but one builtin name can't be a prefix or another so that won't work. It creates the function unconditionally during -i, because there is no way to know if the run-time will end up using it or not. I think this is OK. PL/pgSQL is installed by default in all supported versions. If someone has gone through the bother of uninstalling it, I don't see a need to accommodate them here. I get nearly a 3 fold speed up using the new transaction, from 9184 to 26383 TPS, on 8 CPU machine using scale 50 and: PGOPTIONS="-c synchronous_commit=off" pgbench -c32 -j32 -T60 -b tpcb-like I think this should be committed as a built-in, not just a user-defined transaction, because I would like to see it widely used. In fact, if it weren't for historical consistency I would say it should be the default transaction. Wanting to measure IPC overhead is a valid thing to do, but certainly isn't the most common thing people want to do with pgbench. If a user is limited by IO, it wouldn't matter which transaction they use, and if they are not limited by IO then this transaction is more likely to be the right one for them than the current default one transaction is. Also, as a user-defined transaction with -f, you have to go out of your way to create the function (no "-i" support) and to make sure :scale gets set correctly during runs (as it won't be automatically read from pgbench_branches table, you have manually give -D). Cheers, Jeff pgbench_function_v1.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] subscription worker signalling wal writer too much
On Mon, Jul 3, 2017 at 8:26 PM, Jeff Janes wrote: > On Sat, Jun 24, 2017 at 5:09 PM, Andres Freund wrote: > >> On 2017-06-15 15:06:43 -0700, Jeff Janes wrote: >> >> >> > >> > Wouldn't it >> > better, and much simpler, just to have reverted the first change once >> the >> > second one was done? >> >> I think both can actually happen independently, no? It's pretty common >> for the page lsn to be *older* than the corresponding commit. In fact >> you'll always hit that case unless there's also concurrent writes also >> touching said page. >> > > That is true, but I don't see any residual regression when going from > pre-db76b1efbbab2441428 to 7975c5e0a992ae9a4-with-my-patch applied. So I > think the hint bit issue only shows up when hitting the same page > aggressively, in which case the LSN does get advanced quickly enough that > db76b1efbba solves it. Perhaps a different test case could show that > issue. I also didn't see any improvement from the original 4de82f7d7c50a81e, > so maybe 8 CPUs just isn't enough to detect the problem with setting > hint-buts with permanent tables with synchronous_commit=false. > I've refined my benchmarking, using a smaller scaling factor (8, same as my CPU count) and running the tests for longer. The smaller size means there are more intensive updates on individual pgbench_accounts pages, and the longer run times allows be unset hint bits to build up for longer (usually I like short test runs with a large number of randomized repetitions, but that doesn't work well here). Since the scale factor is the same as the number of CPUs, I've bumped the thread count so that it is more likely someone will choose a non-contended value of pgbench_branches.bid to update at any given moment. pgbench -c16 -j16 -T1200 -M prepared -b tpcb-func --startup='set synchronous_commit=false' This testing did show the importance of waking up the wal writer so that hint bits can be set: commit TPS cfafd8beadcd6f 22200.48 cfafd8beadcd6f_nowake 30766.83 median of 14 runs reported, p-val on difference of means is 2e-22. Where nowake is a hackish disabling of the wake up introduced in 4de82f7d7c50a8, forward ported to 9.6 branch. (I still wake it if is is doing the big sleep, I just took out the part that wake it up even from small sleeps) So my revised benchmark is capable of detecting this effect, even with only 8 CPUs. When I move to next commit where we set hint bits as long as the page was re-dirtied after, get these numbers: db76b1efbbab24 30742.69 db76b1efbbab24_nowake 31072.97 The difference between these is not statistically different, nor is the difference between them and cfafd8beadcd6f. So I think the logic you introduced in db76b1efbbab24 captures all the gain there is to be captured, and 4de82f7d7c50a8 can be reverted. The only reason to wake up the wal writer is if it is taking the big sleep. Of course, I can't prove a negative. There could always be some test case which demonstrates that 4de82f7d7c50a8 still matters in spite of db76b1efbbab24. So to be ultra-conservative, attached is a patch which should preserve all wake-ups other than the ones known to be useless. 7975c5e0a 27810.66 7975c5e0a_patched 30821.41 So 7975c5 causes a significant regression (10% regression, p-val 7e-16 on difference of the means). It repeatedly wakes the wal-writer up to flush a page which the wal-writer simply refuses to flush. This can't serve any purpose. My patch still wakes it up to write the page if it has not been written yet, and also still wakes it to flush a range of pages of WalWriterFlushAfter is met. But it just doesn't wake it up to flush a page which has already been written and will not get flushed. I'd prefer the code-simplifying change of just not waking it up anymore except for when it is doing the big sleep, but I can't reliably detect a performance difference between that and the more conservative change posted here. Cheers, Jeff change_walwriter_wakeup_v2.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch: add --if-exists to pg_recvlogical
On Fri, Aug 25, 2017 at 12:34 PM, Robert Haas wrote: > On Sun, May 21, 2017 at 3:04 PM, Rosser Schwarz > wrote: > > While doing some scripting around pg_recvlogical at $work, I found a need > > for $subject. Attached, find a patch to that effect... Please add this to commitfest.postgresql.org Done, thanks! https://commitfest.postgresql.org/14/1256/ -- :wq
[HACKERS] Back-branch release notes up for review
I've drafted notes for next week's brown-paper-bag releases. If you want to review, see https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=f1b10496a55a64b2872633850e55a2cd9d1c9108 regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [COMMITTERS] pgsql: pg_test_timing: Add NLS
On 7/6/17 14:56, Alvaro Herrera wrote: > We (well, Carlos Chapi, who's doing the translation work now) just > noticed that this has a bug in this line > > + printf("%6s %10s %10s\n", _("< us"), _("% of total"), _("count")); > > _() marks the strings with the c-string flag, which means that the > %-specifiers are checked by gettext, but the % in the third literal is > not a printf specifier. So there's no correct way to write the > translation. We need to use a different xgettext trigger there, one > that doesn't set c-format, but I don't know what. I have fixed this. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Variable substitution in psql backtick expansion
Fabien COELHO writes: > Spending developer time to write code for the hypothetical someone running > a psql version 11 linked to a libpq < 7.4, if it can even link, does not > look like a very good investment... Anyway, here is required the update. The question is the server's version, not libpq. Modern psql does still talk to ancient servers (I tried 11devel against 7.2 just now, to be sure). The stuff in describe.c may not work well, but basic functionality is there and I don't want to break 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] psql - add ability to test whether a variable exists
Here is a version with the :{?varname} syntax. It looks much better for me. I'll admit that it looks better to me as well:-) -- 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] Variable substitution in psql backtick expansion
Hello Tom, I think you are taking unreasonable shortcuts here: + SetVariable(pset.vars, "SERVER_VERSION_NAME", PQparameterStatus(pset.db, "server_version")); The existing code in connection_warnings() does this: const char *server_version; /* Try to get full text form, might include "devel" etc */ server_version = PQparameterStatus(pset.db, "server_version"); /* Otherwise fall back on pset.sversion */ if (!server_version) { formatPGVersionNumber(pset.sversion, true, sverbuf, sizeof(sverbuf)); server_version = sverbuf; } and I think you should duplicate that logic verbatim. Now admittedly, server_version has been available for a long time, so that this might never matter in practice. But we shouldn't be doing this one way in one place and differently somewhere else. Hmmm. I think this code may have been justified around version 6/7. This code could probably be removed: according to the online documentation, "server_version" seems supported at least back to 7.4. Greping old sources suggest that it is not implemented in 7.3, though. Spending developer time to write code for the hypothetical someone running a psql version 11 linked to a libpq < 7.4, if it can even link, does not look like a very good investment... Anyway, here is required the update. -- Fabien.diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml index c592eda..c611f39 100644 --- a/doc/src/sgml/ref/psql-ref.sgml +++ b/doc/src/sgml/ref/psql-ref.sgml @@ -3688,6 +3688,18 @@ bar +SERVER_VERSION_NAME +SERVER_VERSION_NUM + + +These variables are set when connected to reflects the server's version +both in short name (eg "9.6.2", "10.0") and number (eg 90602, 10). +They can be changed or unset. + + + + + SINGLELINE @@ -3733,10 +3745,13 @@ bar VERSION +VERSION_NAME +VERSION_NUM -This variable is set at program start-up to -reflect psql's version. It can be changed or unset. +These variables are set at program start-up to reflect +psql's version in verbose, short name ("10.0") +and number (10). They can be changed or unset. diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c index 96f3a40..b58d8b6 100644 --- a/src/bin/psql/command.c +++ b/src/bin/psql/command.c @@ -3337,6 +3337,9 @@ checkWin32Codepage(void) void SyncVariables(void) { + char vbuf[32]; + const char *server_version; + /* get stuff from connection */ pset.encoding = PQclientEncoding(pset.db); pset.popt.topt.encoding = pset.encoding; @@ -3348,6 +3351,19 @@ SyncVariables(void) SetVariable(pset.vars, "PORT", PQport(pset.db)); SetVariable(pset.vars, "ENCODING", pg_encoding_to_char(pset.encoding)); + /* server version information */ + server_version = PQparameterStatus(pset.db, "server_version"); + /* Otherwise fall back on pset.sversion for libpq < 7.4 */ + if (!server_version) + { + formatPGVersionNumber(pset.sversion, true, vbuf, sizeof(vbuf)); + server_version = vbuf; + } + SetVariable(pset.vars, "SERVER_VERSION_NAME", server_version); + + snprintf(vbuf, sizeof(vbuf), "%d", pset.sversion); + SetVariable(pset.vars, "SERVER_VERSION_NUM", vbuf); + /* send stuff to it, too */ PQsetErrorVerbosity(pset.db, pset.verbosity); PQsetErrorContextVisibility(pset.db, pset.show_context); @@ -3366,6 +3382,8 @@ UnsyncVariables(void) SetVariable(pset.vars, "HOST", NULL); SetVariable(pset.vars, "PORT", NULL); SetVariable(pset.vars, "ENCODING", NULL); + SetVariable(pset.vars, "SERVER_VERSION_NAME", NULL); + SetVariable(pset.vars, "SERVER_VERSION_NUM", NULL); } diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c index 7f76797..4065539 100644 --- a/src/bin/psql/startup.c +++ b/src/bin/psql/startup.c @@ -161,6 +161,8 @@ main(int argc, char *argv[]) EstablishVariableSpace(); SetVariable(pset.vars, "VERSION", PG_VERSION_STR); + SetVariable(pset.vars, "VERSION_NAME", PG_VERSION); + SetVariable(pset.vars, "VERSION_NUM", CppAsString2(PG_VERSION_NUM)); /* Default values for variables (that don't match the result of \unset) */ SetVariableBool(pset.vars, "AUTOCOMMIT"); -- 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] psql - add ability to test whether a variable exists
On Sat, Aug 26, 2017 at 2:09 PM, Pavel Stehule wrote: > > > 2017-08-26 19:55 GMT+02:00 Fabien COELHO : > >> >> Any colon prefixed syntax can be made to work because it is enough for >>> the lexer to detect and handle... so >>> >>> :{defined varname} >>> >>> may be an option, although I do not like the space much because it adds >>> some fuzzyness in the lexer which has to process it. It is probably doable, >>> though. I like having a "?" because there is a question. Other >>> suggestions somehow in line with your proposal could be >>> :{?varname} >>> :{varname?} >>> what do you think? >>> >> >> Here is a version with the :{?varname} syntax. > > > It looks much better for me. > > Regards > > Pavel > +1. Glad to have this feature. Any of the proposed syntaxes look good to me, with a slight preference for {?var}.
Re: [HACKERS] psql - add ability to test whether a variable exists
2017-08-26 19:55 GMT+02:00 Fabien COELHO : > > Any colon prefixed syntax can be made to work because it is enough for the >> lexer to detect and handle... so >> >> :{defined varname} >> >> may be an option, although I do not like the space much because it adds >> some fuzzyness in the lexer which has to process it. It is probably doable, >> though. I like having a "?" because there is a question. Other >> suggestions somehow in line with your proposal could be >> :{?varname} >> :{varname?} >> what do you think? >> > > Here is a version with the :{?varname} syntax. It looks much better for me. Regards Pavel > > > -- > Fabien.
Re: [HACKERS] psql - add ability to test whether a variable exists
Any colon prefixed syntax can be made to work because it is enough for the lexer to detect and handle... so :{defined varname} may be an option, although I do not like the space much because it adds some fuzzyness in the lexer which has to process it. It is probably doable, though. I like having a "?" because there is a question. Other suggestions somehow in line with your proposal could be :{?varname} :{varname?} what do you think? Here is a version with the :{?varname} syntax. -- Fabien.diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml index c592eda..58f8e9a 100644 --- a/doc/src/sgml/ref/psql-ref.sgml +++ b/doc/src/sgml/ref/psql-ref.sgml @@ -781,6 +781,10 @@ testdb=> The forms :'variable_name' and :"variable_name" described there work as well. +The :{?variable_name} syntax allows +to test whether a variable is defined. It is substituted by +TRUE or FALSE. +Escaping the colon with a backslash protects it from substitution. @@ -3813,6 +3817,12 @@ testdb=> INSERT INTO my_table VALUES (:'content'); +The :{?name} special syntax returns TRUE +or FALSE depending on whether the variable exists or not, thus is +always substituted, unless the colon is backslash-escaped. + + + The colon syntax for variables is standard SQL for embedded query languages, such as ECPG. The colon syntaxes for array slices and type casts are diff --git a/src/bin/psql/psqlscanslash.l b/src/bin/psql/psqlscanslash.l index db7a1b9..9a53cb3 100644 --- a/src/bin/psql/psqlscanslash.l +++ b/src/bin/psql/psqlscanslash.l @@ -281,6 +281,10 @@ other . unquoted_option_chars = 0; } +:\{\?{variable_char}+\} { + psqlscan_test_variable(cur_state, yytext, yyleng); +} + :'{variable_char}* { /* Throw back everything but the colon */ yyless(1); @@ -295,6 +299,20 @@ other . ECHO; } +:\{\?{variable_char}* { + /* Throw back everything but the colon */ + yyless(1); + unquoted_option_chars++; + ECHO; +} + +:\{ { + /* Throw back everything but the colon */ + yyless(1); + unquoted_option_chars++; + ECHO; +} + {other} { unquoted_option_chars++; ECHO; diff --git a/src/fe_utils/psqlscan.l b/src/fe_utils/psqlscan.l index 27689d7..4375142a 100644 --- a/src/fe_utils/psqlscan.l +++ b/src/fe_utils/psqlscan.l @@ -745,9 +745,13 @@ other . PQUOTE_SQL_IDENT); } +:\{\?{variable_char}+\} { + psqlscan_test_variable(cur_state, yytext, yyleng); +} + /* * These rules just avoid the need for scanner backup if one of the - * two rules above fails to match completely. + * three rules above fails to match completely. */ :'{variable_char}* { @@ -762,6 +766,17 @@ other . ECHO; } +:\{\?{variable_char}* { + /* Throw back everything but the colon */ + yyless(1); + ECHO; +} +:\{ { + /* Throw back everything but the colon */ + yyless(1); + ECHO; +} + /* * Back to backend-compatible rules. */ @@ -1442,3 +1457,28 @@ psqlscan_escape_variable(PsqlScanState state, const char *txt, int len, psqlscan_emit(state, txt, len); } } + +void +psqlscan_test_variable(PsqlScanState state, const char *txt, int len) +{ + char *varname; + char *value; + + varname = psqlscan_extract_substring(state, txt + 3, len - 4); + if (state->callbacks->get_variable) + value = state->callbacks->get_variable(varname, PQUOTE_PLAIN, + state->cb_passthrough); + else + value = NULL; + free(varname); + + if (value != NULL) + { + psqlscan_emit(state, "TRUE", 4); + free(value); + } + else + { + psqlscan_emit(state, "FALSE", 5); + } +} diff --git a/src/include/fe_utils/psqlscan_int.h b/src/include/fe_utils/psqlscan_int.h index c70ff29..e9b3517 100644 --- a/src/include/fe_utils/psqlscan_int.h +++ b/src/include/fe_utils/psqlscan_int.h @@ -142,5 +142,7 @@ extern char *psqlscan_extract_substring(PsqlScanState state, extern void psqlscan_escape_variable(PsqlScanState state, const char *txt, int len, PsqlScanQuoteType quote); +extern void psqlscan_test_variable(PsqlScanState state, + const char *txt, int len); #endif /* PSQLSCAN_INT_H */ diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected/psql.out index 4aaf4c1..2b2f23b 100644 --- a/src/test/regress/expected/psql.out +++ b/src/test/regress/expected/psql.out @@ -2929,6 +2929,32 @@ bar 'bar' "bar" \echo 'should print #8-1' should print #8-1 \endif +-- :{?...} defined variable test +\set i 1 +\if :{?i} + \echo '#9-1 ok, variable i is defined' +#9-1 ok, variable i is defined +\else + \echo 'should not print #9-2' +\endif +\if :{?no_such_variable} + \echo 'should not print #10-1' +\else + \echo '#10-2 ok, variable no_such_variable is not defined' +#10-2 ok, variable no_such_variable is not defined +\endif +SELECT :{?i} AS i_is_defined; + i_is_defined +-- +
Re: [HACKERS] Build failure on thrips
> woodlouse does not do ecpg-check, and Tom is right anyway. I need to > get a large sign nailed to my forehead that says "Postgres is C, and > declarations must come first." I didn't notice either. I guess /me is getting rusty, sigh. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL -- 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] Build failure on thrips
> Unless someone has a better idea, I'd suggest working around > that like so: > > #ifdef WIN32 > #ifdef _MSC_VER/* requires MSVC */ > _configthreadlocale(_ENABLE_PER_THREAD_LOCALE); > #endif > #endif Let's try, committed, thanks again Tom. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL -- 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] Build failure on thrips
* Michael Meskes wrote: >> It's not just thrips. Of the Windows machines that have reported in >> since >> that commit, only currawong and baiji passed. Although lorikeet, >> woodlouse, bowerbird, and brolga are showing green, this proves >> nothing >> because they're all configured to skip the ecpg check (... I wonder >> why). > > I would assume it works on woodlouse as the patch has been submitted by > woodlouse's owner. woodlouse does not do ecpg-check, and Tom is right anyway. I need to get a large sign nailed to my forehead that says "Postgres is C, and declarations must come first." I'm not sure, and not able to find out right this moment, why thrips' compiler does not accept this code now, because I tested it on thrips (and woodlouse, IIRC) before I sent it in. -- Christian -- 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] Variable substitution in psql backtick expansion
Fabien COELHO writes: > So basically the only thing needed from Robert & you seems to change > "11.0" to "11devel", which is fine with me. > The attached v5 does that. I think you are taking unreasonable shortcuts here: + SetVariable(pset.vars, "SERVER_VERSION_NAME", PQparameterStatus(pset.db, "server_version")); The existing code in connection_warnings() does this: const char *server_version; /* Try to get full text form, might include "devel" etc */ server_version = PQparameterStatus(pset.db, "server_version"); /* Otherwise fall back on pset.sversion */ if (!server_version) { formatPGVersionNumber(pset.sversion, true, sverbuf, sizeof(sverbuf)); server_version = sverbuf; } and I think you should duplicate that logic verbatim. Now admittedly, server_version has been available for a long time, so that this might never matter in practice. But we shouldn't be doing this one way in one place and differently somewhere else. 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] Build failure on thrips
> It's not just thrips. Of the Windows machines that have reported in > since > that commit, only currawong and baiji passed. Although lorikeet, > woodlouse, bowerbird, and brolga are showing green, this proves > nothing > because they're all configured to skip the ecpg check (... I wonder > why). I would assume it works on woodlouse as the patch has been submitted by woodlouse's owner. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL -- 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] Variable substitution in psql backtick expansion
Hello Tom, I understand that you would prefer VERSION_NAME to show something like "11devel, server 9.6.4" No, that's not what I said. I'm just complaining that as the patch stands it will set SERVER_NAME to "11.0", where I think it should say "11devel" (as of today). Ok. [...] VERSION "PostgreSQL 11devel on ..." CLIENT_VERSION_NAME "11devel" CLIENT_VERSION_NUM 11 This kind of inconsistencies is hard for human memory:-( or just leaving "CLIENT" implicit for all of these variables: VERSION "PostgreSQL 11devel on ..." VERSION_NAME "11devel" VERSION_NUM 11 That is already what the patch does, because of the VERSION precedent. Robert seems to prefer the last of those, and that'd be fine with me. (Note that CLIENT is ambiguous anyway: does it mean psql itself, or libpq?) Hmmm. Indeed. SERVER_VERSION_NAME "9.6.4" SERVER_VERSION_NUM 090604 I'm on board with this, except I don't think we should have any leading zero in the numeric form. There are contexts where somebody might think that means octal. Indeed. The implementation already does this, I just typed it without checking. So basically the only thing needed from Robert & you seems to change "11.0" to "11devel", which is fine with me. The attached v5 does that. -- Fabien.diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml index c592eda..c611f39 100644 --- a/doc/src/sgml/ref/psql-ref.sgml +++ b/doc/src/sgml/ref/psql-ref.sgml @@ -3688,6 +3688,18 @@ bar +SERVER_VERSION_NAME +SERVER_VERSION_NUM + + +These variables are set when connected to reflects the server's version +both in short name (eg "9.6.2", "10.0") and number (eg 90602, 10). +They can be changed or unset. + + + + + SINGLELINE @@ -3733,10 +3745,13 @@ bar VERSION +VERSION_NAME +VERSION_NUM -This variable is set at program start-up to -reflect psql's version. It can be changed or unset. +These variables are set at program start-up to reflect +psql's version in verbose, short name ("10.0") +and number (10). They can be changed or unset. diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c index 96f3a40..3c1924e 100644 --- a/src/bin/psql/command.c +++ b/src/bin/psql/command.c @@ -3337,6 +3337,8 @@ checkWin32Codepage(void) void SyncVariables(void) { + char vbuf[32]; + /* get stuff from connection */ pset.encoding = PQclientEncoding(pset.db); pset.popt.topt.encoding = pset.encoding; @@ -3348,6 +3350,11 @@ SyncVariables(void) SetVariable(pset.vars, "PORT", PQport(pset.db)); SetVariable(pset.vars, "ENCODING", pg_encoding_to_char(pset.encoding)); + /* server version information */ + SetVariable(pset.vars, "SERVER_VERSION_NAME", PQparameterStatus(pset.db, "server_version")); + snprintf(vbuf, sizeof(vbuf), "%d", pset.sversion); + SetVariable(pset.vars, "SERVER_VERSION_NUM", vbuf); + /* send stuff to it, too */ PQsetErrorVerbosity(pset.db, pset.verbosity); PQsetErrorContextVisibility(pset.db, pset.show_context); @@ -3366,6 +3373,8 @@ UnsyncVariables(void) SetVariable(pset.vars, "HOST", NULL); SetVariable(pset.vars, "PORT", NULL); SetVariable(pset.vars, "ENCODING", NULL); + SetVariable(pset.vars, "SERVER_VERSION_NAME", NULL); + SetVariable(pset.vars, "SERVER_VERSION_NUM", NULL); } diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c index 7f76797..4065539 100644 --- a/src/bin/psql/startup.c +++ b/src/bin/psql/startup.c @@ -161,6 +161,8 @@ main(int argc, char *argv[]) EstablishVariableSpace(); SetVariable(pset.vars, "VERSION", PG_VERSION_STR); + SetVariable(pset.vars, "VERSION_NAME", PG_VERSION); + SetVariable(pset.vars, "VERSION_NUM", CppAsString2(PG_VERSION_NUM)); /* Default values for variables (that don't match the result of \unset) */ SetVariableBool(pset.vars, "AUTOCOMMIT"); -- 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] Build failure on thrips
I wrote: > thrips is showing some weird syntax errors that I suspect mean it's > misinterpreting "_configthreadlocale(_ENABLE_PER_THREAD_LOCALE);" > as a declaration. Actually, looking closer, isn't the problem that you've stuck a function call into the midst of declarations? Some compilers will allow that, but it's not valid C90. You probably just need to move the call down past "EXEC SQL END DECLARE SECTION". > So what it looks like to me is that either that function isn't > available on all Windows versions, or there's some other header > you need to include to get it on some versions. Microsoft's own documentation contradicts that: https://msdn.microsoft.com/en-us/library/26c0tb7x(v=vs.80).aspx So I'm not sure why the undefined-function results on frogmouth and jacana, although I notice they are using gcc not MSVC. Unless someone has a better idea, I'd suggest working around that like so: #ifdef WIN32 #ifdef _MSC_VER/* requires MSVC */ _configthreadlocale(_ENABLE_PER_THREAD_LOCALE); #endif #endif 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] Build failure on thrips
Michael Meskes writes: > The bug fix I committed for fixing setlocale in ECPG test cases on Windows > seems to work nicely on all buildfarm animals except thrips. Could anyone with > access to a similar systems please check what's going on or send the > precompiled file that creates the compilation errors? It's not just thrips. Of the Windows machines that have reported in since that commit, only currawong and baiji passed. Although lorikeet, woodlouse, bowerbird, and brolga are showing green, this proves nothing because they're all configured to skip the ecpg check (... I wonder why). In the rest, we have frogmouth: thread_implicit.pgc: In function 'test_thread': thread_implicit.pgc:104:2: warning: implicit declaration of function '_configthreadlocale' thread_implicit.pgc:104:22: error: '_ENABLE_PER_THREAD_LOCALE' undeclared (first use in this function) jacana: Aug 26 09:06:40 thread_implicit.o: In function `test_thread': Aug 26 09:06:40 c:\mingw\msys\1.0\home\pgrunner\bf\root\HEAD\pgsql.build\src\interfaces\ecpg\test\thread/thread_implicit.pgc:104: undefined reference to `_configthreadlocale' thrips is showing some weird syntax errors that I suspect mean it's misinterpreting "_configthreadlocale(_ENABLE_PER_THREAD_LOCALE);" as a declaration. So what it looks like to me is that either that function isn't available on all Windows versions, or there's some other header you need to include to get it on some versions. 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] psql - add ability to test whether a variable exists
Hello Pavel, As a follow-up to the \if patch by Corey Huinker, here is a proposal to allow testing whether a client-side variable exists in psql. The syntax is as ugly as the current :'var' and :"var" things, but ISTM that this is the only simple option to have a working SQL-compatible syntax with both client-side substitution and server side execution. See the second example below. It is really ugly Yes, I agree:-) - the ? symbol is not used in pair usually - so it is much more visible - it is bad readable. Yep. Maybe some other syntax: :{fx xxx} .. where fx can be one from more possible operators ? ! ... Any colon prefixed syntax can be made to work because it is enough for the lexer to detect and handle... so :{defined varname} may be an option, although I do not like the space much because it adds some fuzzyness in the lexer which has to process it. It is probably doable, though. I like having a "?" because there is a question. Other suggestions somehow in line with your proposal could be :{?varname} :{varname?} what do you think? The other option would be to have some special keyword syntax, say "defined var", but then it would have to be parsed client side, and how to do that in an SQL expression is unclear, and moreover it would not look right in an SQL expression. If it would look like a function call, say "defined('var')", it would potentially interact with existing server-side user-defined functions, which is pretty undesirable. Hence the :?...? proposal above which is limited to variable subsitution syntax. should not be solved by introduction \ifdef ? This would be a client side only non extendable option: you can only test one variable at a time, you cannot say "NOT" or have to do \ifndef... CPP started like that and ended with #if bool-expr-with-defined in the end. The idea is to extend the newly added \if with client-side SQL-compatible expression syntax, and that the same syntax could be used server side with select, eg: -- client side \let i :j + 1 -- server side SELECT :j + 1 AS i \gset -- client-side conditional \if NOT :j > 1 OR ... The colon prefixed substitution syntax already works for server side, but there is no way to check whether a variable exists which is compatible with that, hence this patch. Pgbench has expressions with patches to improve it, especially adding boolean operators. I think that the simplest plan is, when stabalized, to move the parser & executir to fe_utils and to use it from both psql et pgbench. Also I plan to add \if to pgbench, possibly by factoring some of the code from psql in fe_utils as well because it would help with benchmarks. Now given the patch queue length and speed I'm not even thinking of starting doing all this. -- 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] Variable substitution in psql backtick expansion
Fabien COELHO writes: >> OK, but if human-friendly display is the use-case then it ought to >> duplicate what psql itself would print in, eg, the startup message about >> server version mismatch. The v4 patch does not, in particular it neglects >> PQparameterStatus(pset.db, "server_version"). This would result in >> printing, eg, "11.0" when the user would likely rather see "11devel". > I understand that you would prefer VERSION_NAME to show something like >"11devel, server 9.6.4" No, that's not what I said. I'm just complaining that as the patch stands it will set SERVER_NAME to "11.0", where I think it should say "11devel" (as of today). > In summary, my prefered option is to have: >CLIENT_VERSION "PostgreSQL 11devel on ..." >CLIENT_VERSION_NAME "11devel" >CLIENT_VERSION_NUM 11 I don't think we want to drop :VERSION; that would accomplish little beyond breaking existing scripts. Plausible choices include duplicating it, like: VERSION "PostgreSQL 11devel on ..." CLIENT_VERSION "PostgreSQL 11devel on ..." CLIENT_VERSION_NAME "11devel" CLIENT_VERSION_NUM 11 or just ignoring the discrepancy: VERSION "PostgreSQL 11devel on ..." CLIENT_VERSION_NAME "11devel" CLIENT_VERSION_NUM 11 or just leaving "CLIENT" implicit for all of these variables: VERSION "PostgreSQL 11devel on ..." VERSION_NAME "11devel" VERSION_NUM 11 Robert seems to prefer the last of those, and that'd be fine with me. (Note that CLIENT is ambiguous anyway: does it mean psql itself, or libpq?) >SERVER_VERSION_NAME "9.6.4" >SERVER_VERSION_NUM 090604 I'm on board with this, except I don't think we should have any leading zero in the numeric form. There are contexts where somebody might think that means octal. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PATCH: multivariate histograms and MCV lists
On 08/17/2017 12:06 PM, Adrien Nayrat wrote:> > Hello, > > There is no check of "statistics type/kind" in > pg_stats_ext_mcvlist_items and pg_histogram_buckets. > > select stxname,stxkind from pg_statistic_ext ; stxname | stxkind > ---+- stts3 | {h} stts2 | {m} > > So you can call : > > SELECT * FROM pg_mcv_list_items((SELECT oid FROM pg_statistic_ext > WHERE stxname = 'stts3')); > > SELECT * FROM pg_histogram_buckets((SELECT oid FROM pg_statistic_ext > WHERE stxname = 'stts2'), 0); > > Both crashes. > Thanks for the report, this is clearly a bug. I don't think we need to test the stxkind, but rather a missing check that the requested type is actually built. > Unfotunately, I don't have the knowledge to produce a patch :/ > > Small fix in documentation, patch attached. > Thanks, will fix. regards -- Tomas Vondra http://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] hash partitioning based on v10Beta2
Hi all, Now we have had the range / list partition, but hash partitioning is not implemented yet. Attached is a POC patch based on the v10Beta2 to add the hash partitioning feature. Although we will need more discussions about the syntax and other specifications before going ahead the project, but I think this runnable code might help to discuss what and how we implement this. Description The hash partition's implement is on the basis of the original range / list partition,and using similar syntax. To create a partitioned table ,use: CREATE TABLE h (id int) PARTITION BY HASH(id); The partitioning key supports only one value, and I think the partition key can support multiple values, which may be difficult to implement when querying, but it is not impossible. A partition table can be create as bellow: CREATE TABLE h1 PARTITION OF h; CREATE TABLE h2 PARTITION OF h; CREATE TABLE h3 PARTITION OF h; FOR VALUES clause cannot be used, and the partition bound is calclulated automatically as partition index of single integer value. An inserted record is stored in a partition whose index equals DatumGetUInt32(OidFunctionCall1(lookup_type_cache(key->parttypid[0], TYPECACHE_HASH_PROC)->hash_proc, values[0])) % nparts /* Number of partitions */ ; In the above example, this is DatumGetUInt32(OidFunctionCall1(lookup_type_cache(key->parttypid[0], TYPECACHE_HASH_PROC)->hash_proc, id)) % 3; postgres=# insert into h select generate_series(1,20); INSERT 0 20 postgres=# select tableoid::regclass,* from h; tableoid | id --+ h1 | 3 h1 | 5 h1 | 17 h1 | 19 h2 | 2 h2 | 6 h2 | 7 h2 | 11 h2 | 12 h2 | 14 h2 | 15 h2 | 18 h2 | 20 h3 | 1 h3 | 4 h3 | 8 h3 | 9 h3 | 10 h3 | 13 h3 | 16 (20 rows) The number of partitions here can be dynamically added, and if a new partition is created, the number of partitions changes, the calculated target partitions will change, and the same data is not reasonable in different partitions,So you need to re-calculate the existing data and insert the target partition when you create a new partition. postgres=# create table h4 partition of h; CREATE TABLE postgres=# select tableoid::regclass,* from h; tableoid | id --+ h1 | 5 h1 | 17 h1 | 19 h1 | 6 h1 | 12 h1 | 8 h1 | 13 h2 | 11 h2 | 14 h3 | 1 h3 | 9 h3 | 2 h3 | 15 h4 | 3 h4 | 7 h4 | 18 h4 | 20 h4 | 4 h4 | 10 h4 | 16 (20 rows) When querying the data, the hash partition uses the same algorithm as the insertion, and filters out the table that does not need to be scanned. postgres=# explain analyze select * from h where id = 1; QUERY PLAN Append (cost=0.00..41.88 rows=13 width=4) (actual time=0.020..0.023 rows=1 loops=1) -> Seq Scan on h3 (cost=0.00..41.88 rows=13 width=4) (actual time=0.013..0.016 rows=1 loops=1) Filter: (id = 1) Rows Removed by Filter: 3 Planning time: 0.346 ms Execution time: 0.061 ms (6 rows) postgres=# explain analyze select * from h where id in (1,5);; QUERY PLAN Append (cost=0.00..83.75 rows=52 width=4) (actual time=0.016..0.028 rows=2 loops=1) -> Seq Scan on h1 (cost=0.00..41.88 rows=26 width=4) (actual time=0.015..0.018 rows=1 loops=1) Filter: (id = ANY ('{1,5}'::integer[])) Rows Removed by Filter: 6 -> Seq Scan on h3 (cost=0.00..41.88 rows=26 width=4) (actual time=0.005..0.007 rows=1 loops=1) Filter: (id = ANY ('{1,5}'::integer[])) Rows Removed by Filter: 3 Planning time: 0.720 ms Execution time: 0.074 ms (9 rows) postgres=# explain analyze select * from h where id = 1 or id = 5;; QUERY PLAN Append (cost=0.00..96.50 rows=50 width=4) (actual time=0.017..0.078 rows=2 loops=1) -> Seq Scan on h1 (cost=0.00..48.25 rows=25 width=4) (actual time=0.015..0.019 rows=1 loops=1) Filter: ((id = 1) OR (id = 5)) Rows Removed by Filter: 6 -> Seq Scan on h3 (cost=0.00..48.25 rows=25 width=4) (actual time=0.005..0.010 rows=1 loops=1) Filter: ((id = 1) OR (id = 5)) Rows Removed by Filter: 3 Planning time: 0.396 ms Execution time: 0.139 ms (9 rows) Can not detach / attach / drop partition table. Be
[HACKERS] Build failure on thrips
The bug fix I committed for fixing setlocale in ECPG test cases on Windows seems to work nicely on all buildfarm animals except thrips. Could anyone with access to a similar systems please check what's going on or send the precompiled file that creates the compilation errors? Thanks. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL -- 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] Not listed as committer
> Ha, that's interesting. > > Should be fixed now, please try again. Works, thanks Magnus. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL -- 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] ECPG: WHENEVER statement with DO CONTINUE action
> Given that it's Friday evening in Europe, I'm betting Michael is gone > for the day. In the interests of getting the buildfarm back to > green, > I'll see if I can fix this. Correct, thanks Tom. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: [PATCH] Fix drop replication slot blocking instead of returning error
On Thu, Aug 24, 2017 at 03:38:20PM +0200, Simone Gotti wrote: > I noticed that in postgres 10beta3, calling pg_drop_replication_slot on an > active slot will block until it's released instead of returning an error > like > done in pg 9.6. Since this is a change in the previous behavior and the docs > wasn't changed I made a patch to restore the previous behavior. > > Thanks, > > Simone. > > -- > > after git commit 9915de6c1cb calls to pg_drop_replication_slot or the > replication protocol DROP_REPLICATION_SLOT command will block when a > slot is in use instead of returning an error as before (as the doc > states). > > This patch will set nowait to true to restore the previous > behavior. [Action required within three days. This is a generic notification.] The above-described topic is currently a PostgreSQL 10 open item. Álvaro, since you committed the patch believed to have created it, you own this open item. If some other commit is more relevant or if this does not belong as a v10 open item, please let us know. Otherwise, please observe the policy on open item ownership[1] and send a status update within three calendar days of this message. Include a date for your subsequent status update. Testers may discover new open items at any time, and I want to plan to get them all fixed well in advance of shipping v10. Consequently, I will appreciate your efforts toward speedy resolution. Thanks. [1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] psql - add ability to test whether a variable exists
2017-08-26 8:54 GMT+02:00 Fabien COELHO : > > Hello, > > As a follow-up to the \if patch by Corey Huinker, here is a proposal to > allow testing whether a client-side variable exists in psql. > > The syntax is as ugly as the current :'var' and :"var" things, but ISTM > that this is the only simple option to have a working SQL-compatible syntax > with both client-side substitution and server side execution. See the > second example below. > It is really ugly - the ? symbol is not used in pair usually - so it is much more visible - it is bad readable. Maybe some other syntax: :{fx xxx} .. where fx can be one from more possible operators ? ! ... > > -- client side use > psql> \set i 1 > psql> \if :?i? > psql> \echo 'i is defined' > psql> \endif > -- use server-side in an SQL expression > psql> SELECT NOT :?VERSION_NUM? OR :'VERSION' <> VERSTION() AS bad_conf > \gset > > psql> \if :bad_conf \echo 'too bad...' \quit \endif > > The other option would be to have some special keyword syntax, say > "defined var", but then it would have to be parsed client side, and how to > do that in an SQL expression is unclear, and moreover it would not look > right in an SQL expression. If it would look like a function call, say > "defined('var')", it would potentially interact with existing server-side > user-defined functions, which is pretty undesirable. Hence the :?...? > proposal above which is limited to variable subsitution syntax. should not be solved by introduction \ifdef ? > > -- > Fabien. > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers > >