Re: project updates
Having tried David's method to install 10.4 and 11 on my mac and turns out
worked for me. The compiling issue posted by Aleksander is because
some json helpers changed function name and is not backward compatible with
9.4 and 10. Using #if macro resolves the problem,
Here is the commit to solve this.
https://github.com/charles-cui/pg_thrift/commit/dd5b8ad17ab47e3c977943dd2d69e5abad34c6ad
Aleksander, do you want to test again?
2018-07-21 13:08 GMT-07:00 Charles Cui :
> yes, using home brew, will try that.
>
> On Jul 21, 2018 12:33 PM, "David Fetter" wrote:
>
> On Sat, Jul 21, 2018 at 12:00:48PM -0700, Charles Cui wrote:
> > 2018-07-20 2:18 GMT-07:00 Aleksander Alekseeev <
> a.aleks...@postgrespro.ru>:
> >
> > > Hello Charles,
> > >
> > > > Here is my current working status.
> > > > 1. Complete the thrift_binary_in and thrift_binary_out functions, so
> > > > that users can express their thrift struct using json. These two
> > > > functions support both simple data struct and complex data structure
> > > > like struct and map. 2. added test cases and document to cover
> > > > thrift_binary_in and thrift_binary_out. 3. make the code compile-able
> > > > from 9.4 to 11.0.
> > >
> > > I see multiple warnings during compilation with 11.0, e.g:
> > >
> > > ```
> > > pg_thrift.c:1120:72: warning: implicit declaration of function
> > > ‘JsonbGetDatum’; did you mean ‘JsonbPGetDatum’?
> > > [-Wimplicit-function-declaration]
> > >
> > > pg_thrift.c:1206:13: warning: the address of ‘is_big_endian’ will
> > > always evaluate as ‘true’ [-Waddress]
> > >
> > > pg_thrift.c:1227:18: warning: implicit declaration of function
> > > ‘PG_GETARG_JSONB’; did you mean ‘PG_GETARG_JSONB_P’?
> > > [-Wimplicit-function-declaration]
> > >
> > > pg_thrift.c:1227:18: warning: initialization of ‘Jsonb *’ {aka ‘struct
> > > *’} from ‘int’ makes pointer from integer without a cast
> > > [-Wint-conversion]
> > > ```
> > >
> > > Also tests (make clean && make && make install && make installcheck)
> > > don't pass.
> > >
> > I have tested compiled passed for 9.4, 9.5, 9.6, 10 and 11 (
> > https://travis-ci.org/charles-cui/pg_thrift/builds/404741899)
> > and make install && make installcheck passed for 9.4
> > The tests are not run for other versions because my mac is 9.4
> postgresql.
>
> You can have several versions of PostgreSQL on your mac at once. Are
> you using homebrew?
>
> Best,
> David.
>
> --
> David Fetter http://fetter.org/
> Phone: +1 415 235 3778
>
> Remember to vote!
> Consider donating to Postgres: http://www.postgresql.org/about/donate
>
>
>
Re: pg_ugprade test failure on data set with column with default value with type bit/varbit
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: tested, passed
Documentation:not tested
Hi Paul,
this is a review of the patch:
CABQrizc90sfkZgi4=+0bbp1zu3yex9sm4rjbe1yncvzf3qk...@mail.gmail.com
There hasn't been any problem, at least that I've been able to find.
This one applies cleanly.
Compile, pg_upgrade and pg_dumpall passed without error too.
Follow below a comparison of the results of the pg_dumpall:
# Without patch #
...
CREATE TABLE public.t111 (
a40 bit varying(5) DEFAULT (B'1'::"bit")::bit varying
);
...
CREATE TABLE public.t222 (
a40 bit varying(5) DEFAULT B'1'::"bit"
);
# With patch #
...
CREATE TABLE public.t111 (
a40 bit varying(5) DEFAULT ('1'::"bit")::bit varying
);
...
CREATE TABLE public.t222 (
a40 bit varying(5) DEFAULT '1'::"bit"
);
The "B", used to indicated a bit-string constant, removed as expected.
+1 for committer review
--
Davy Machado
pgbench: improve --help and --version parsing
Hi,
There is a small catch in the parsing of --help and --version args by pgbench:
they are processed correctly only as the first argument. If it's not the case,
strange error message occurs:
$ pgbench -q --help
pgbench: unrecognized option '--help'
Try "pgbench --help" for more information.
The reason for this behavior is how these two arguments are handled in
src/bin/pgbench/pgbench.c:
if (argc > 1)
{
if (strcmp(argv[1], "--help") == 0 || strcmp(argv[1], "-?") == 0)
{
usage();
exit(0);
}
if (strcmp(argv[1], "--version") == 0 || strcmp(argv[1], "-V") == 0)
{
puts("pgbench (PostgreSQL) " PG_VERSION);
exit(0);
}
}
All other arguments are processed with getopt_long. The proposed patch replaces
the existing way of parsing the --help and --version arguments with getopt_long,
expanding the existing switch statement.
--
Andrei Korigodski
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 41b756c089..d1ff85a677 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -4763,6 +4763,7 @@ main(int argc, char **argv)
{"define", required_argument, NULL, 'D'},
{"file", required_argument, NULL, 'f'},
{"fillfactor", required_argument, NULL, 'F'},
+ {"help", no_argument, NULL, '?'},
{"host", required_argument, NULL, 'h'},
{"initialize", no_argument, NULL, 'i'},
{"init-steps", required_argument, NULL, 'I'},
@@ -4783,6 +4784,7 @@ main(int argc, char **argv)
{"transactions", required_argument, NULL, 't'},
{"username", required_argument, NULL, 'U'},
{"vacuum-all", no_argument, NULL, 'v'},
+ {"version", no_argument, NULL, 'V'},
/* long-named only options */
{"unlogged-tables", no_argument, NULL, 1},
{"tablespace", required_argument, NULL, 2},
@@ -4832,20 +4834,6 @@ main(int argc, char **argv)
progname = get_progname(argv[0]);
- if (argc > 1)
- {
- if (strcmp(argv[1], "--help") == 0 || strcmp(argv[1], "-?") == 0)
- {
- usage();
- exit(0);
- }
- if (strcmp(argv[1], "--version") == 0 || strcmp(argv[1], "-V") == 0)
- {
- puts("pgbench (PostgreSQL) " PG_VERSION);
- exit(0);
- }
- }
-
#ifdef WIN32
/* stderr is buffered on Win32. */
setvbuf(stderr, NULL, _IONBF, 0);
@@ -4868,7 +4856,7 @@ main(int argc, char **argv)
exit(1);
}
- while ((c = getopt_long(argc, argv, "iI:h:nvp:dqb:SNc:j:Crs:t:T:U:lf:D:F:M:P:R:L:", long_options, )) != -1)
+ while ((c = getopt_long(argc, argv, "iI:h:nvp:dqb:SNc:j:Crs:t:T:U:lf:D:F:M:P:R:L:V?", long_options, )) != -1)
{
char *script;
@@ -5097,6 +5085,14 @@ main(int argc, char **argv)
latency_limit = (int64) (limit_ms * 1000);
}
break;
+ case 'V':
+puts("pgbench (PostgreSQL) " PG_VERSION);
+exit(0);
+break;
+ case '?':
+usage();
+exit(0);
+break;
case 1:/* unlogged-tables */
initialization_option_set = true;
unlogged_tables = true;
Re: Remove psql's -W option
Vik Fearing writes: > On 22/07/18 00:41, Fabien COELHO wrote: >> What is the rational? > It's first on our list of things not to do: > https://wiki.postgresql.org/wiki/Don't_Do_This#Don.27t_use_psql_-W_or_--password As I recall, when this has been discussed in the past, people objected because they didn't like either (1) the extra server process fork and/or network round trip caused when a password is needed, or (2) the server log entry that gets generated about client disconnecting without supplying a password. (We don't log anything about it normally, but I'm not sure that that's always true when using PAM, LDAP, connection poolers, etc.) While those are surely niche concerns, it's not really apparent to me what we gain by breaking them. A possible positive reason for removing the option would be if we could clean up this mess: https://www.postgresql.org/message-id/e1egdgc-000302...@gemulon.postgresql.org But no fair citing that argument without presenting an actual clean-up patch, because it's not obvious how much cleaner we could make it. BTW, all of our client programs have this switch, so if we did agree to remove it, this patch doesn't go nearly far enough. regards, tom lane PS: I found some interesting back-story here: https://www.postgresql.org/message-id/flat/200712091148.54294.xzilla%40users.sourceforge.net
Re: Non-portable shell code in pg_upgrade tap tests
"Tels" writes: > Looking at your new patch, I notice you used "" for quoting, not ''. (Not > sure which variant Tom used when pushing a patch). > I'm not a shell expert, but I think '' are safer, as "" still has some > interpolation from the shell (at least on the systems I use regulary): We can't do that here because '' would suppress interpolation of the variable's value, which is sort of the point. AFAIK, the locution "$foo" is safe regardless of what is in $foo, as long as only one pass of shell evaluation is involved. The shell will treat the value of $foo as one not-further-interpreted command argument. regards, tom lane
Re: Remove psql's -W option
On 22/07/18 00:41, Fabien COELHO wrote: > > Hello David, > >> I'd like to $Subject for 12. >> >> There are scripts it could break, but not ones that weren't already >> broken in ways important to access control. >> >> What say? > > What is the rational? It's first on our list of things not to do: https://wiki.postgresql.org/wiki/Don't_Do_This#Don.27t_use_psql_-W_or_--password -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Re: Non-portable shell code in pg_upgrade tap tests
Moin,
On Sat, July 21, 2018 12:47 pm, Dagfinn Ilmari Mannsåker wrote:
> Tom Lane writes:
>
>> "Tels" writes:
>>> + *) if [ `find ${PGDATA} -type f ! -perm 640 | wc -l` -ne 0 ]; then
>>
>>> Shouldn't ${PGDATA} in the above as argument to find be quoted,
>>> otherwise
>>> the shell would get confused if it contains spaces or other special
>>> characters?
>>
>> Hmm. Yeah, probably. I don't think this script is meant to be run with
>> arbitrary values of PGDATA, but most of the other uses are quoted, so
>> for consistency's sake this should be too.
>
> PGDATA is built from `pwd`, so it breaks if the build directory has a
> space in it. Because it's testing for the absence of files, it doesn't
> actually break the test, but would fail to detect the bugs it's trying
> to.
Thanx for the fix. But perhaps I should have been a bit more specific in
my first message, spaces are not the only character this can break at.
Looking at your new patch, I notice you used "" for quoting, not ''. (Not
sure which variant Tom used when pushing a patch).
I'm not a shell expert, but I think '' are safer, as "" still has some
interpolation from the shell (at least on the systems I use regulary):
te@pc:~$ mkdir 'test$test'
te@pc:~$ touch 'test$test/test'
te@pc:~$ find 'test$test/test' -type f
test$test/test
te@pc:~$ find "test$test/test" -type f
find: ‘test/test’: No such file or directory
So I've grown the habit to always use '' instead of "". Not sure if this
is specific to bash vs. sh or PC vs Mac, tho.
Best wishes,
Tels
Re: Remove psql's -W option
Hello David, I'd like to $Subject for 12. There are scripts it could break, but not ones that weren't already broken in ways important to access control. What say? What is the rational? I'm unsure of the logic behind removing -W (--password) but keeping -w (--no-password), especially as the internal logic seems kept by the patch. -- Fabien.
Re: Remove psql's -W option
On 21/07/18 23:58, David Fetter wrote: > Folks, > > I'd like to $Subject for 12. > > There are scripts it could break, but not ones that weren't already > broken in ways important to access control. > > What say? I say it should at least throw a sensible error for a few versions before it's removed completely. -1 on this patch +1 for removing the "feature" -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Remove psql's -W option
Folks,
I'd like to $Subject for 12.
There are scripts it could break, but not ones that weren't already
broken in ways important to access control.
What say?
Best,
David.
--
David Fetter http://fetter.org/
Phone: +1 415 235 3778
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
>From f3382131fd2172eed32052231d4d4ec968bd2c6d Mon Sep 17 00:00:00 2001
From: David Fetter
Date: Sat, 21 Jul 2018 14:26:59 -0700
Subject: [PATCH] Remove the -W option from psql (v01)
To: pgsql-hack...@postgresql.org
EFOOTGUN
---
doc/src/sgml/ref/psql-ref.sgml | 25 -
src/bin/psql/help.c| 1 -
src/bin/psql/startup.c | 6 +-
3 files changed, 1 insertion(+), 31 deletions(-)
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index b17039d60f..c0ef7102d0 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -503,31 +503,6 @@ EOF
-
- -W
- --password
-
-
- Force psql to prompt for a
- password before connecting to a database.
-
-
-
- This option is never essential, since psql
- will automatically prompt for a password if the server demands
- password authentication. However, psql
- will waste a connection attempt finding out that the server wants a
- password. In some cases it is worth typing -W to avoid
- the extra connection attempt.
-
-
-
- Note that this option will remain set for the entire session,
- and so it affects uses of the meta-command
- \connect as well as the initial connection attempt.
-
-
-
-x
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index 702e742af4..cd7614364c 100644
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -138,7 +138,6 @@ usage(unsigned short int pager)
env = user;
fprintf(output, _(" -U, --username=USERNAME database user name (default: \"%s\")\n"), env);
fprintf(output, _(" -w, --no-passwordnever prompt for password\n"));
- fprintf(output, _(" -W, --password force password prompt (should happen automatically)\n"));
fprintf(output, _("\nFor more information, type \"\\?\" (for internal commands) or \"\\help\" (for SQL\n"
"commands) from within psql, or consult the psql section in the PostgreSQL\n"
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index be57574cd3..432a583583 100644
--- a/src/bin/psql/startup.c
+++ b/src/bin/psql/startup.c
@@ -464,7 +464,6 @@ parse_psql_options(int argc, char *argv[], struct adhoc_opts *options)
{"variable", required_argument, NULL, 'v'},
{"version", no_argument, NULL, 'V'},
{"no-password", no_argument, NULL, 'w'},
- {"password", no_argument, NULL, 'W'},
{"expanded", no_argument, NULL, 'x'},
{"no-psqlrc", no_argument, NULL, 'X'},
{"help", optional_argument, NULL, 1},
@@ -476,7 +475,7 @@ parse_psql_options(int argc, char *argv[], struct adhoc_opts *options)
memset(options, 0, sizeof *options);
- while ((c = getopt_long(argc, argv, "aAbc:d:eEf:F:h:HlL:no:p:P:qR:sStT:U:v:VwWxXz?01",
+ while ((c = getopt_long(argc, argv, "aAbc:d:eEf:F:h:HlL:no:p:P:qR:sStT:U:v:VwxXz?01",
long_options, )) != -1)
{
switch (c)
@@ -615,9 +614,6 @@ parse_psql_options(int argc, char *argv[], struct adhoc_opts *options)
case 'w':
pset.getPassword = TRI_NO;
break;
- case 'W':
-pset.getPassword = TRI_YES;
-break;
case 'x':
pset.popt.topt.expanded = true;
break;
--
2.17.1
Re: [HACKERS] Bug in to_timestamp().
On Sun, Jul 8, 2018 at 12:15 AM Alexander Korotkov < a.korot...@postgrespro.ru> wrote: > On Sat, Jul 7, 2018 at 12:31 AM Tom Lane wrote: > > Alexander Korotkov writes: > > > I tool a look at this patch. It looks good for me. It applies > > > cleanly on last master, builds without warnings, passes tests. > > > Functionality seems to be well-covered by documentation and regression > > > tests. So, I'm going to commit it if no objections. > > > > AFAIR, the argument was mainly over whether we agreed with the proposed > > behavioral changes. It seems a bit premature to me to commit given that > > there's not consensus on that. > > Ok. I've briefly looked to the thread, and I thought there is > consensus already. Given your concern, I'll investigate this thread > in detail and come up with summary. > So, I've studies this thread in more details. Let me share my short summary. This thread was started from complaint about confusing behavior of to_timestamp() function in some cases. Basically confusing behavior is related to two issues: interpretation of spaces and separators in both input string and format string, tolerance to invalid dates and times (i.e. 2009-06-40 becomes 2009-07-10). Fix for the second issue was already committed by Tom Lane (d3cd36a1). So, the improvement under consideration is interpretation of spaces and separators. to_timestamp()/to_date() functions are not part of SQL standard. So, unfortunately we can't use standard as a guideline and have to search for other criteria. On the one hand to_timestamp()/to_date() is here for quite long time and there might be existing applications relying on its behavior. Changing the behavior of these functions might appear to be a pain for users. On the other hand these functions were introduced basically for Oracle compatibility. So it would be nice to keep their behavior as close to Oracle as possible. On the third hand, if current behavior of these functions is agreed to be confusing, and new behavior is agreed to be less confusing and more close to Oracle, then we can accept it. If even it would discourage small fraction of users, who are relying on the behavior which we assume to be confusing, way larger fraction of users would be encouraged by this change. So, as I get from this thread, current patch brings these function very close to Oracle behavior. The only divergence found yet is related to handling of unmatched tails of input strings (Oracle throws an error while we swallow that silently) [1]. But this issue can be be addressed by a separate patch. Patch seems to be in a pretty good shape. So, the question is whether there is a consensus that we want a backward-incompatible behavior change, which this patch does. My personal opinion is +1 for committing this, because I see that this patch takes a lot of community efforts on discussion, coding, review etc. All these efforts give a substantial result in a patch, which makes behavior more Oracle-compatible and (IMHO) more clear. However, in this thread other opinions were expressed. In particular, David G. Johnston expressed opinion that we shouldn't change behavior of existing functions, alternatively we could introduce new functions with new behavior. However, I see David doesn't participate this thread for a quite long time. Thus, in the current situation I can propose following. Let's establish some period when people can express objections against committing this patch (reasonable short period, say 1 week). If no objections were expressed then commit. Otherwise, discuss the objections expressed. 1. https://www.postgresql.org/message-id/CA%2Bq6zcUS3fQGLoeLcuJLtz-gRD6vHqpn1fBe0cnORdx93QtO4w%40mail.gmail.com -- Alexander Korotkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
Re: JIT breaks PostGIS
Hi,
Here's somewhat minimized example.
https://gist.github.com/Komzpa/cc3762175328ff5d11de4b972352003d
You can put this file into regress/jitbug.sql in PostGIS code tree and run
after building postgis:
perl regress/run_test.pl regress/jitbug.sql --expect
perl regress/run_test.pl regress/jitbug.sql
сб, 21 июл. 2018 г. в 23:39, Andres Freund :
> Hi,
>
> On 2018-07-21 22:36:44 +0200, Christoph Berg wrote:
> > Re: Andres Freund 2018-07-21 <
> 20180721202543.ri5jyfclj6kb6...@alap3.anarazel.de>
> > > Could you attempt to come up with a smaller reproducer?
> >
> > The original instructions in
> > https://trac.osgeo.org/postgis/ticket/4125 are fairly easy to follow;
> > there's also a postgresql-11-postgis-2.5{,-scripts} package (not
> > mentioned in there) that exhibits the bug.
>
> Sure, but a more minimal example (than a 1kloc regression script, wiht
> possible inter statement dependencies) still makes the debugging
> easier...
>
> Greetings,
>
> Andres Freund
>
--
Darafei Praliaskouski
Support me: http://patreon.com/komzpa
Re: pgbench-ycsb
Could you provide a link to the specification?
I cannot find something simple, and I was kind of hoping to avoid diving
into the source code of the java tool on github:-) In particular, I'm
looking for a description of the expected underlying schema and its size
(scale) parameters.
There are the description files for different workloads, like [1], (with the
custom amount of records, of course) and the schema [2]. Would this
information be enough?
[1]: https://github.com/brianfrankcooper/YCSB/blob/master/workloads/workloada
[2]:
https://github.com/brianfrankcooper/YCSB/blob/master/jdbc/src/main/resources/sql/create_table.sql
The second link is a start.
I notice that the submitted patch transactions do not apply to this
schema, which is significantly different from the pgbench TPC-B (like)
benchmark.
The YCSB schema is key -> fields[0-9], all of them TEXT, somehow expected
to be 100 bytes each, and update is expected to update one of these
fields.
This suggest that maybe a -i extension would be in order. Possibly
pgbench -i -s 1 --layout={tpcb,ycsb} (or schema ?)
where "tpcb" would be the default?
I'm sceptical about using a textual primary key as it corresponds more to
NoSQL limitations than to an actual design choice. I'd be okay with INT8
as a pkey.
I find the YSCB tablename "usertable" especially unhelpful. Maybe
"pgbench_ycsb"?
--
Fabien.
Re: JIT breaks PostGIS
Hi,
On 2018-07-21 22:36:44 +0200, Christoph Berg wrote:
> Re: Andres Freund 2018-07-21
> <20180721202543.ri5jyfclj6kb6...@alap3.anarazel.de>
> > Could you attempt to come up with a smaller reproducer?
>
> The original instructions in
> https://trac.osgeo.org/postgis/ticket/4125 are fairly easy to follow;
> there's also a postgresql-11-postgis-2.5{,-scripts} package (not
> mentioned in there) that exhibits the bug.
Sure, but a more minimal example (than a 1kloc regression script, wiht
possible inter statement dependencies) still makes the debugging
easier...
Greetings,
Andres Freund
Re: JIT breaks PostGIS
Re: Andres Freund 2018-07-21 <20180721202543.ri5jyfclj6kb6...@alap3.anarazel.de>
> Could you attempt to come up with a smaller reproducer?
The original instructions in
https://trac.osgeo.org/postgis/ticket/4125 are fairly easy to follow;
there's also a postgresql-11-postgis-2.5{,-scripts} package (not
mentioned in there) that exhibits the bug.
(Add "stretch-pgdg-testing main 11" to sources.list.)
That said, I'm just rebuilding postgresql-11 on stretch to use
llvm-4.0 instead of 3.9.
Christoph
Re: Make foo=null a warning by default.
On Sat, Jul 21, 2018 at 04:23:14PM -0400, Fabien COELHO wrote: > > Hello David, > > >>[...] > > > >Fixed. > > > >>[...] > > > >I've changed it to something that makes more sense to me. Hope it > >makes more sense to others. > > Ok, all is fine for me. > > I could not find a CF entry, so I created one: > > https://commitfest.postgresql.org/19/1728/ Thanks! Best, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
Re: JIT breaks PostGIS
Hi, On 2018-07-21 23:14:47 +0300, Darafei "Komяpa" Praliaskouski wrote: > Today I spent some time closing PostGIS tickets in preparation to Monday's > release. > > One of the blockers, https://trac.osgeo.org/postgis/ticket/4125, was filed > by Postgres APT repository maintainer Christoph Berg who noticed a test > suite failure on Debian Stretch with Postgres 11. > > Upon investigation we found: > - A build for Ubuntu Bionic installed on Debian Stretch passes the suite, > requiring llvm6; > - A build for Debian Stretch fails the suite on a call to external library > GEOS, showing no traces of JIT in the stacktrace; > - Setting jit=off lets the suite pass; > - The query called in clean session by itself does not crash Postgres. > Queries above it are required to reproduce the crash; > - The crash affects not only Stretch, but customly collected Postgres 11 / > clang 3.9 on Travis CI running Ubuntu Trusty: > https://github.com/postgis/postgis/pull/262. > > I suspect that a fix would require to bisect llvm/clang version which stops > showing this behavior and making it a new minimum for JIT, if this is not a > symptom of bigger (memory management?) problem. Could you attempt to come up with a smaller reproducer? Greetings, Andres Freund
Re: Make foo=null a warning by default.
Hello David, [...] Fixed. [...] I've changed it to something that makes more sense to me. Hope it makes more sense to others. Ok, all is fine for me. I could not find a CF entry, so I created one: https://commitfest.postgresql.org/19/1728/ -- Fabien.
Re: Possible performance regression in version 10.1 with pgbench read-write tests.
Andres Freund writes: > On 2018-07-20 16:43:33 -0400, Tom Lane wrote: >> On my RHEL6 machine, with unmodified HEAD and 8 sessions (since I've >> only got 8 cores) but other parameters matching Mithun's example, >> I just got > It's *really* common to have more actual clients than cpus for oltp > workloads, so I don't think it's insane to test with more clients. I finished a set of runs using similar parameters to Mithun's test except for using 8 clients, and another set using 72 clients (but, being impatient, 5-minute runtime) just to verify that the results wouldn't be markedly different. I got TPS numbers like this: 8 clients 72 clients unmodified HEAD 16112 16284 with padding patch 16096 16283 with SysV semas 15926 16064 with padding+SysV 15949 16085 This is on RHEL6 (kernel 2.6.32-754.2.1.el6.x86_64), hardware is dual 4-core Intel E5-2609 (Sandy Bridge era). This hardware does show NUMA effects, although no doubt less strongly than Mithun's machine. I would like to see some other results with a newer kernel. I tried to repeat this test on a laptop running Fedora 28, but soon concluded that anything beyond very short runs was mainly going to tell me about thermal throttling :-(. I could possibly get repeatable numbers from, say, 1-minute SELECT-only runs, but that would be a different test scenario, likely one with a lot less lock contention. Anyway, for me, the padding change is a don't-care. Given that both Mithun and Thomas showed some positive effect from it, I'm not averse to applying it. I'm still -1 on going back to SysV semas. regards, tom lane
JIT breaks PostGIS
Hi, Today I spent some time closing PostGIS tickets in preparation to Monday's release. One of the blockers, https://trac.osgeo.org/postgis/ticket/4125, was filed by Postgres APT repository maintainer Christoph Berg who noticed a test suite failure on Debian Stretch with Postgres 11. Upon investigation we found: - A build for Ubuntu Bionic installed on Debian Stretch passes the suite, requiring llvm6; - A build for Debian Stretch fails the suite on a call to external library GEOS, showing no traces of JIT in the stacktrace; - Setting jit=off lets the suite pass; - The query called in clean session by itself does not crash Postgres. Queries above it are required to reproduce the crash; - The crash affects not only Stretch, but customly collected Postgres 11 / clang 3.9 on Travis CI running Ubuntu Trusty: https://github.com/postgis/postgis/pull/262. I suspect that a fix would require to bisect llvm/clang version which stops showing this behavior and making it a new minimum for JIT, if this is not a symptom of bigger (memory management?) problem. Thank you! -- Darafei Praliaskouski Support me: http://patreon.com/komzpa
Re: project updates
yes, using home brew, will try that.
On Jul 21, 2018 12:33 PM, "David Fetter" wrote:
On Sat, Jul 21, 2018 at 12:00:48PM -0700, Charles Cui wrote:
> 2018-07-20 2:18 GMT-07:00 Aleksander Alekseeev :
>
> > Hello Charles,
> >
> > > Here is my current working status.
> > > 1. Complete the thrift_binary_in and thrift_binary_out functions, so
> > > that users can express their thrift struct using json. These two
> > > functions support both simple data struct and complex data structure
> > > like struct and map. 2. added test cases and document to cover
> > > thrift_binary_in and thrift_binary_out. 3. make the code compile-able
> > > from 9.4 to 11.0.
> >
> > I see multiple warnings during compilation with 11.0, e.g:
> >
> > ```
> > pg_thrift.c:1120:72: warning: implicit declaration of function
> > ‘JsonbGetDatum’; did you mean ‘JsonbPGetDatum’?
> > [-Wimplicit-function-declaration]
> >
> > pg_thrift.c:1206:13: warning: the address of ‘is_big_endian’ will
> > always evaluate as ‘true’ [-Waddress]
> >
> > pg_thrift.c:1227:18: warning: implicit declaration of function
> > ‘PG_GETARG_JSONB’; did you mean ‘PG_GETARG_JSONB_P’?
> > [-Wimplicit-function-declaration]
> >
> > pg_thrift.c:1227:18: warning: initialization of ‘Jsonb *’ {aka ‘struct
> > *’} from ‘int’ makes pointer from integer without a cast
> > [-Wint-conversion]
> > ```
> >
> > Also tests (make clean && make && make install && make installcheck)
> > don't pass.
> >
> I have tested compiled passed for 9.4, 9.5, 9.6, 10 and 11 (
> https://travis-ci.org/charles-cui/pg_thrift/builds/404741899)
> and make install && make installcheck passed for 9.4
> The tests are not run for other versions because my mac is 9.4 postgresql.
You can have several versions of PostgreSQL on your mac at once. Are
you using homebrew?
Best,
David.
--
David Fetter http://fetter.org/
Phone: +1 415 235 3778
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
Re: project updates
On Sat, Jul 21, 2018 at 12:00:48PM -0700, Charles Cui wrote:
> 2018-07-20 2:18 GMT-07:00 Aleksander Alekseeev :
>
> > Hello Charles,
> >
> > > Here is my current working status.
> > > 1. Complete the thrift_binary_in and thrift_binary_out functions, so
> > > that users can express their thrift struct using json. These two
> > > functions support both simple data struct and complex data structure
> > > like struct and map. 2. added test cases and document to cover
> > > thrift_binary_in and thrift_binary_out. 3. make the code compile-able
> > > from 9.4 to 11.0.
> >
> > I see multiple warnings during compilation with 11.0, e.g:
> >
> > ```
> > pg_thrift.c:1120:72: warning: implicit declaration of function
> > ‘JsonbGetDatum’; did you mean ‘JsonbPGetDatum’?
> > [-Wimplicit-function-declaration]
> >
> > pg_thrift.c:1206:13: warning: the address of ‘is_big_endian’ will
> > always evaluate as ‘true’ [-Waddress]
> >
> > pg_thrift.c:1227:18: warning: implicit declaration of function
> > ‘PG_GETARG_JSONB’; did you mean ‘PG_GETARG_JSONB_P’?
> > [-Wimplicit-function-declaration]
> >
> > pg_thrift.c:1227:18: warning: initialization of ‘Jsonb *’ {aka ‘struct
> > *’} from ‘int’ makes pointer from integer without a cast
> > [-Wint-conversion]
> > ```
> >
> > Also tests (make clean && make && make install && make installcheck)
> > don't pass.
> >
> I have tested compiled passed for 9.4, 9.5, 9.6, 10 and 11 (
> https://travis-ci.org/charles-cui/pg_thrift/builds/404741899)
> and make install && make installcheck passed for 9.4
> The tests are not run for other versions because my mac is 9.4 postgresql.
You can have several versions of PostgreSQL on your mac at once. Are
you using homebrew?
Best,
David.
--
David Fetter http://fetter.org/
Phone: +1 415 235 3778
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
Re: project updates
2018-07-20 2:18 GMT-07:00 Aleksander Alekseeev :
> Hello Charles,
>
> > Here is my current working status.
> > 1. Complete the thrift_binary_in and thrift_binary_out functions, so
> > that users can express their thrift struct using json. These two
> > functions support both simple data struct and complex data structure
> > like struct and map. 2. added test cases and document to cover
> > thrift_binary_in and thrift_binary_out. 3. make the code compile-able
> > from 9.4 to 11.0.
>
> I see multiple warnings during compilation with 11.0, e.g:
>
> ```
> pg_thrift.c:1120:72: warning: implicit declaration of function
> ‘JsonbGetDatum’; did you mean ‘JsonbPGetDatum’?
> [-Wimplicit-function-declaration]
>
> pg_thrift.c:1206:13: warning: the address of ‘is_big_endian’ will
> always evaluate as ‘true’ [-Waddress]
>
> pg_thrift.c:1227:18: warning: implicit declaration of function
> ‘PG_GETARG_JSONB’; did you mean ‘PG_GETARG_JSONB_P’?
> [-Wimplicit-function-declaration]
>
> pg_thrift.c:1227:18: warning: initialization of ‘Jsonb *’ {aka ‘struct
> *’} from ‘int’ makes pointer from integer without a cast
> [-Wint-conversion]
> ```
>
> Also tests (make clean && make && make install && make installcheck)
> don't pass.
>
I have tested compiled passed for 9.4, 9.5, 9.6, 10 and 11 (
https://travis-ci.org/charles-cui/pg_thrift/builds/404741899)
and make install && make installcheck passed for 9.4
The tests are not run for other versions because my mac is 9.4 postgresql.
Also for CI testing, the current travis scripts has some problems for
version other than 9.4.
I will try to upgrade my postgresql and see whether I can make it.
>
> > One question though, for custom types, it seems rich operations are
> > also very important besides storing and expressing using thrift type.
> > What are the most important operators should I support? Any
> > suggestions? Here is the updated repo
> > https://github.com/charles-cui/pg_thrift
>
> I believe get field / set field are most common ones. Naturally there
> are also enumerations, projections, you name it. You can check out list
> of operation supported by JSONB (or a list of operations on dict's in
> Python) to get a full picture.
>
Good idea, thanks
>
> --
> Best regards,
> Aleksander Alekseev
>
Re: Non-portable shell code in pg_upgrade tap tests
ilm...@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes: > Tom Lane writes: >> Hmm. Yeah, probably. I don't think this script is meant to be run with >> arbitrary values of PGDATA, but most of the other uses are quoted, so >> for consistency's sake this should be too. > Attached is a patch fixing this. I checked the rest of the script, and > this seems to be the only place lacking quoting. Done already, but thanks. regards, tom lane
Re: Non-portable shell code in pg_upgrade tap tests
Tom Lane writes:
> "Tels" writes:
>> +*) if [ `find ${PGDATA} -type f ! -perm 640 | wc -l` -ne 0 ]; then
>
>> Shouldn't ${PGDATA} in the above as argument to find be quoted, otherwise
>> the shell would get confused if it contains spaces or other special
>> characters?
>
> Hmm. Yeah, probably. I don't think this script is meant to be run with
> arbitrary values of PGDATA, but most of the other uses are quoted, so
> for consistency's sake this should be too.
PGDATA is built from `pwd`, so it breaks if the build directory has a
space in it. Because it's testing for the absence of files, it doesn't
actually break the test, but would fail to detect the bugs it's trying
to.
+ find /home/ilmari/src/post gresql/src/bin/pg_upgrade/tmp_check/data -type f !
-perm 640
+ wc -l
find: ‘/home/ilmari/src/post’: No such file or directory
find: ‘gresql/src/bin/pg_upgrade/tmp_check/data’: No such file or directory
+ [ 0 -ne 0 ]
+ find /home/ilmari/src/post gresql/src/bin/pg_upgrade/tmp_check/data -type d !
-perm 750
+ wc -l
find: ‘/home/ilmari/src/post’: No such file or directory
find: ‘gresql/src/bin/pg_upgrade/tmp_check/data’: No such file or directory
+ [ 0 -ne 0 ]
Attached is a patch fixing this. I checked the rest of the script, and
this seems to be the only place lacking quoting.
- ilmari
--
"The surreality of the universe tends towards a maximum" -- Skud's Law
"Never formulate a law or axiom that you're not prepared to live with
the consequences of." -- Skud's Meta-Law
>From cabd43aa1988fb9f33743981266c9bf2278681a2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?=
Date: Sat, 21 Jul 2018 17:42:39 +0100
Subject: [PATCH] Quote ${PGDATA} in pg_upgrade/test.sh
---
src/bin/pg_upgrade/test.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/bin/pg_upgrade/test.sh b/src/bin/pg_upgrade/test.sh
index 775dd5729d..0e285c5c17 100644
--- a/src/bin/pg_upgrade/test.sh
+++ b/src/bin/pg_upgrade/test.sh
@@ -234,7 +234,7 @@ pg_upgrade $PG_UPGRADE_OPTS -d "${PGDATA}.old" -D "${PGDATA}" -b "$oldbindir" -B
# Windows hosts don't support Unix-y permissions.
case $testhost in
MINGW*) ;;
- *) if [ `find ${PGDATA} -type f ! -perm 640 | wc -l` -ne 0 ]; then
+ *) if [ `find "${PGDATA}" -type f ! -perm 640 | wc -l` -ne 0 ]; then
echo "files in PGDATA with permission != 640";
exit 1;
fi ;;
@@ -242,7 +242,7 @@ esac
case $testhost in
MINGW*) ;;
- *) if [ `find ${PGDATA} -type d ! -perm 750 | wc -l` -ne 0 ]; then
+ *) if [ `find "${PGDATA}" -type d ! -perm 750 | wc -l` -ne 0 ]; then
echo "directories in PGDATA with permission != 750";
exit 1;
fi ;;
--
2.18.0
grammar - src/backend/access/heap/README.tuplock
The sentences in question currently read ...
Finally, SELECT FOR KEY SHARE obtains a shared lock which only
prevents tuple removal and modifications of key fields. This last mode
implements a mode just strong enough to implement RI checks, i.e. it ensures
that tuples do not go away from under a check, without blocking when some
other transaction that want to update the tuple without changing its key.
The last sentence is the problem, the first is just for context.
1) "This last mode implements a mode ..." seems awkward and the term "mode"
is not used to describe the other 3 levels of tuple locking strength. The
phrasing
used for SELECT FOR UPDATE ("This is the lock level that ...") is better.
2) noun/verb agreement
... transaction that want ...
should be either
... transaction that wants ... or ... transactions that want ...
3) mismatched clauses
... without blocking when ... transactions that want ...
should be either
... without blocking ... transactions that want ...
or
... without blocking when ... transactions want ...
The first says that our transaction's shared lock is not blocking other
transactions.
4) the "some other" in "some other transaction" just seems unnecessary.
Putting it all together, the attached patch changes the last sentence to ...
This lock level is just strong enough to implement RI checks, i.e. it
ensures
that tuples do not go away from under a check, without blocking transactions
that want to update the tuple without changing its key.
README.tuplock.patch
Description: Binary data
Re: Non-portable shell code in pg_upgrade tap tests
"Tels" writes:
> + *) if [ `find ${PGDATA} -type f ! -perm 640 | wc -l` -ne 0 ]; then
> Shouldn't ${PGDATA} in the above as argument to find be quoted, otherwise
> the shell would get confused if it contains spaces or other special
> characters?
Hmm. Yeah, probably. I don't think this script is meant to be run with
arbitrary values of PGDATA, but most of the other uses are quoted, so
for consistency's sake this should be too.
regards, tom lane
Re: Have an encrypted pgpass file
Sorry Tom (and others), I didn't notice my change affected libpq. Though I would really like the possibility to have an encrypted way of presenting the pgpassfile. Would it be more secure if the script / command is passed as a compile option to libpg and the variable only contains the filename of the encrypted file or only the arguments to pass to the command and use the original pgpassfile to decrypt. As always the security of the library / command in the hands of the person who compiles it. So assume the library is compiled with PGPASSDECRYPTCOMMAND=/usr/bin/gpg PGPASSFILE=pgpass.gpg PGPASSARGUMENTS="-q -d" In the end libpq would call: '/usr/bin/gpg -q -d pgpass.gpg' The only thing I'm wondering, is it flexible enough for use cases different than mine? Or should I make a static variable for it so the user of the libpq can define it if they want to use the feature, and if not defined ignore the feature? I can make a new patch, if this is the direction we want to go. Best regards, Marco van Eck On Sat, Jul 21, 2018 at 7:29 AM Tom Lane wrote: > Isaac Morland writes: > >>> It would also provide a *very* fertile source of shell-script-injection > >>> vulnerabilities. (Whaddya mean, you tried to use a user name with a > >>> quote mark in it?) > > > If I understand the proposal correctly, the pgpass program would run on > the > > client, invoked by libpq when a password is needed for a connection. So > the > > risk relates to strange things happening on the client when the client > > attempts to connect as a strangely-named user or to a strangely-named > > database or host, not to being able to break into the server. > > Yeah. The most obvious scenario for trouble is that somebody enters > a crafted user name on a website, and that results in bad things happening > on an application-server machine that tried to pass that user name to > a database server. The DB server itself isn't compromised, but the app > server could be. > > If we were putting this sort of feature into psql, it wouldn't be such > a risk, but if it's in libpq then I fear it is. libpq underlies a lot > of client-side code. > > regards, tom lane >
Re: GiST VACUUM
Hi! > 19 июля 2018 г., в 23:26, Andrey Borodin написал(а): > > I'm working on triggering left split during vacuum. Will get back when done. > Thanks! Here's patch including some messy hacks to trigger NSN and FollowRight jumps during VACUUM. To trigger FollowRight GiST sometimes forget to clear follow-right marker simulating crash of an insert. This fills logs with "fixing incomplete split" messages. Search for "REMOVE THIS" to disable these ill-behavior triggers. To trigger NSN jump GiST allocate empty page after every real allocation. gistvacuumcleanup() was constantly generating left jumps because there was used 0 instead of real start NSN, I moved NSN acquisition to gistvacuumscan(). Also fixed some comments. gistvacuumcleanup() will have same effect as gistbulkdelete(), is it OK? To reproduce left-jumps run ./rescantest.sh Script contain variables for my local paths. Best regards, Andrey Borodin. 0001-Physical-GiST-scan-in-VACUUM-v13.patch Description: Binary data
Re: Indicate anti-wraparound autovacuum in log_autovacuum_min_duration
Hello > Yes, a bit more verbosity would be nice for that. Using the term > "anti-wraparound" directly in the logs makes the most sense? I used "(to prevent wraparound)" to looks like reporting in pg_stat_activity. As far i can see currently we not using "anti-wraparound" for user messages. On the other hand we using anti-wraparound in docs, so i can change wording. > The patch could be made simpler by using empty strings with a single > ereport() call. How about proper translate in this case? Currently we translate full line "automatic vacuum of table \"%s.%s.%s\": index scans: %d\n" before ereport call. regards, Sergei
Re: Indicate anti-wraparound autovacuum in log_autovacuum_min_duration
On Sat, Jul 21, 2018 at 09:38:38AM +0300, Sergei Kornilov wrote: > Currently log_autovacuum_min_duration log message has no difference > between regular autovacuum and to prevent wraparound autovacuum. There > are important differences, for example, backend can automatically > cancel regular autovacuum, but not anti-wraparound. I think it is > useful indicate anti-wraparound in logs. Yes, a bit more verbosity would be nice for that. Using the term "anti-wraparound" directly in the logs makes the most sense? > Small patch attached. I am not sure can be anti-wraparound autovacuum > not aggressive, so i leave all 4 variants. The patch could be made simpler by using empty strings with a single ereport() call. -- Michael signature.asc Description: PGP signature
Indicate anti-wraparound autovacuum in log_autovacuum_min_duration
Hello
Currently log_autovacuum_min_duration log message has no difference between
regular autovacuum and to prevent wraparound autovacuum. There are important
differences, for example, backend can automatically cancel regular autovacuum,
but not anti-wraparound. I think it is useful indicate anti-wraparound in logs.
Small patch attached. I am not sure can be anti-wraparound autovacuum not
aggressive, so i leave all 4 variants.
regards, Sergeidiff --git a/src/backend/commands/vacuumlazy.c b/src/backend/commands/vacuumlazy.c
index 5649a70..5792854 100644
--- a/src/backend/commands/vacuumlazy.c
+++ b/src/backend/commands/vacuumlazy.c
@@ -374,10 +374,20 @@ lazy_vacuum_rel(Relation onerel, int options, VacuumParams *params,
* emitting individual parts of the message when not applicable.
*/
initStringInfo();
- if (aggressive)
-msgfmt = _("automatic aggressive vacuum of table \"%s.%s.%s\": index scans: %d\n");
+ if (params->is_wraparound)
+ {
+if (aggressive)
+ msgfmt = _("automatic aggressive vacuum (to prevent wraparound) of table \"%s.%s.%s\": index scans: %d\n");
+else
+ msgfmt = _("automatic vacuum (to prevent wraparound) of table \"%s.%s.%s\": index scans: %d\n");
+ }
else
-msgfmt = _("automatic vacuum of table \"%s.%s.%s\": index scans: %d\n");
+ {
+if (aggressive)
+ msgfmt = _("automatic aggressive vacuum of table \"%s.%s.%s\": index scans: %d\n");
+else
+ msgfmt = _("automatic vacuum of table \"%s.%s.%s\": index scans: %d\n");
+ }
appendStringInfo(, msgfmt,
get_database_name(MyDatabaseId),
get_namespace_name(RelationGetNamespace(onerel)),
