Re: pgsql: Allow db.schema.table patterns, but complain about random garbag
On 2022-04-22 Fr 22:59, Noah Misch wrote: > On Sat, Apr 23, 2022 at 09:12:20AM +1200, Thomas Munro wrote: >> On Sat, Apr 23, 2022 at 8:06 AM Robert Haas wrote: >>> I have to say the fact that IPC::Run does shell-glob expansion of its >>> arguments on some machines and not others seems ludicrous to me. This >>> patch may be overtested, but such a radical behavior difference is >>> completely nuts. How is anyone supposed to write reliable tests for >>> any feature in the face of such wildly inconsistent behavior? (I missed seeing the part where I was asked for help earlier on this thread) > The MinGW gcc crt*.o files do shell-glob expansion on the arguments before > entering main(). See https://google.com/search?q=mingw+command+line+glob for > various discussion of that behavior. I suspect you experienced that, not any > IPC::Run behavior. (I haven't tested, though.) Commit 11e9caf likely had the > same cause, though the commit message attributed it to the msys shell rather > than to crt*.o. > > Let's disable that MinGW compiler behavior. > https://willus.com/mingw/_globbing.shtml lists two ways of achieving that. Yeah. I can definitely confirm that this is the proximate cause of the issue, and not either IPC::Run or the shell, which is why all my experiments on this failed. With this patch diff --git a/src/include/port/win32.h b/src/include/port/win32.h index c6213c77c3..456c3f31f1 100644 --- a/src/include/port/win32.h +++ b/src/include/port/win32.h @@ -77,3 +77,7 @@ struct sockaddr_un charsun_path[108]; }; #define HAVE_STRUCT_SOCKADDR_UN 1 + +#ifndef _MSC_VER +extern int _CRT_glob = 0; /* 0 turns off globbing; 1 turns it on */ +#endif fairywren happily passes the tests that Robert has since reverted. I'm rather tempted to call this CRT behaviour a mis-feature, especially as a default. I think we should certainly disable it in the development branch, and consider back-patching it, although it is a slight change in behaviour, albeit one that we didn't know about much less want or document. Still, we been building with mingw compilers for about 20 years and haven't hit this before so far as we know, so maybe not. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
Re: pgsql: Allow db.schema.table patterns, but complain about random garbag
On Sun, Apr 24, 2022 at 01:09:08PM -0400, Andrew Dunstan wrote: > On 2022-04-22 Fr 22:59, Noah Misch wrote: > > The MinGW gcc crt*.o files do shell-glob expansion on the arguments before > > entering main(). See https://google.com/search?q=mingw+command+line+glob > > for > > various discussion of that behavior. I suspect you experienced that, not > > any > > IPC::Run behavior. (I haven't tested, though.) Commit 11e9caf likely had > > the > > same cause, though the commit message attributed it to the msys shell rather > > than to crt*.o. > > > > Let's disable that MinGW compiler behavior. > > https://willus.com/mingw/_globbing.shtml lists two ways of achieving that. > > Yeah. I can definitely confirm that this is the proximate cause of the > issue, and not either IPC::Run or the shell, which is why all my > experiments on this failed. With this patch Thanks for confirming. > diff --git a/src/include/port/win32.h b/src/include/port/win32.h > index c6213c77c3..456c3f31f1 100644 > --- a/src/include/port/win32.h > +++ b/src/include/port/win32.h > @@ -77,3 +77,7 @@ struct sockaddr_un > charsun_path[108]; > }; > #define HAVE_STRUCT_SOCKADDR_UN 1 > + > +#ifndef _MSC_VER > +extern int _CRT_glob = 0; /* 0 turns off globbing; 1 turns it on */ > +#endif > > > fairywren happily passes the tests that Robert has since reverted. > > > I'm rather tempted to call this CRT behaviour a mis-feature, especially > as a default. I think we should certainly disable it in the development > branch, and consider back-patching it, although it is a slight change in > behaviour, albeit one that we didn't know about much less want or > document. Still, we been building with mingw compilers for about 20 > years and haven't hit this before so far as we know, so maybe not. I'd lean toward back-patching. We position MSVC and MinGW as two ways to build roughly the same PostgreSQL, not as two routes to different user-facing behavior. Since the postgresql.org/download binaries use MSVC, aligning with their behavior is good. It's fair to mention in the release notes, of course. Does your win32.h patch build without warnings or errors? Even if MinGW has some magic to make that work, I suspect we'll want a non-header home. Perhaps src/common/exec.c? It's best to keep this symbol out of libpq and other DLLs, though I bet exports.txt would avoid functional problems.
Re: pgsql: Allow db.schema.table patterns, but complain about random garbag
On 2022-04-24 Su 14:19, Noah Misch wrote: > On Sun, Apr 24, 2022 at 01:09:08PM -0400, Andrew Dunstan wrote: >> On 2022-04-22 Fr 22:59, Noah Misch wrote: >>> The MinGW gcc crt*.o files do shell-glob expansion on the arguments before >>> entering main(). See https://google.com/search?q=mingw+command+line+glob >>> for >>> various discussion of that behavior. I suspect you experienced that, not >>> any >>> IPC::Run behavior. (I haven't tested, though.) Commit 11e9caf likely had >>> the >>> same cause, though the commit message attributed it to the msys shell rather >>> than to crt*.o. >>> >>> Let's disable that MinGW compiler behavior. >>> https://willus.com/mingw/_globbing.shtml lists two ways of achieving that. >> Yeah. I can definitely confirm that this is the proximate cause of the >> issue, and not either IPC::Run or the shell, which is why all my >> experiments on this failed. With this patch > Thanks for confirming. > >> diff --git a/src/include/port/win32.h b/src/include/port/win32.h >> index c6213c77c3..456c3f31f1 100644 >> --- a/src/include/port/win32.h >> +++ b/src/include/port/win32.h >> @@ -77,3 +77,7 @@ struct sockaddr_un >> charsun_path[108]; >> }; >> #define HAVE_STRUCT_SOCKADDR_UN 1 >> + >> +#ifndef _MSC_VER >> +extern int _CRT_glob = 0; /* 0 turns off globbing; 1 turns it on */ >> +#endif >> >> >> fairywren happily passes the tests that Robert has since reverted. >> >> >> I'm rather tempted to call this CRT behaviour a mis-feature, especially >> as a default. I think we should certainly disable it in the development >> branch, and consider back-patching it, although it is a slight change in >> behaviour, albeit one that we didn't know about much less want or >> document. Still, we been building with mingw compilers for about 20 >> years and haven't hit this before so far as we know, so maybe not. > I'd lean toward back-patching. We position MSVC and MinGW as two ways to > build roughly the same PostgreSQL, not as two routes to different user-facing > behavior. Since the postgresql.org/download binaries use MSVC, aligning with > their behavior is good. It's fair to mention in the release notes, of course. OK, good point. > Does your win32.h patch build without warnings or errors? Yes. > Even if MinGW has > some magic to make that work, I suspect we'll want a non-header home. Perhaps > src/common/exec.c? It's best to keep this symbol out of libpq and other > DLLs, though I bet exports.txt would avoid functional problems. exec.c looks like it should work fine. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
Re: [RFC] building postgres with meson -v8
On 2022-04-21 Th 17:34, Tom Lane wrote: > Andres Freund writes: >> On 2022-04-21 22:36:01 +0200, Peter Eisentraut wrote: >>> Why is Python 3.5 relevant? >> It's the latest available on some older platforms. It's pretty easy to >> install >> a new meson, a heck of a lot more work to install a new python. IIRC solaris, >> AIX and some of Tom's dinosaurs. > FWIW, I don't think that either gaur or prairiedog need be factored into > this conversation. They cannot build ninja at all for lack of , > so whether they could run meson is pretty much beside the point. > > (I wonder if we should stick in a configure test for , > just to see if anything else doesn't have it?) > > We should worry a little more about Solaris and AIX, but even there I > think it's largely up to the platform owner whether they've updated > python to something modern. If it isn't, you need to move the goalposts > back some more :-(. As of today I see the following pre-3.6 pythons > in the buildfarm, exclusive of mine: > > skate 3.2.3 > snapper 3.2.3 > topminnow 3.4.2 > hornet3.4.3 > sungazer 3.4.3 > wrasse3.4.3 > shelduck 3.4.10 > curculio 3.5.1 > hoverfly 3.5.1 > batfish 3.5.2 > spurfowl 3.5.2 > cuon 3.5.2 > ayu 3.5.3 > chimaera 3.5.3 > chipmunk 3.5.3 > grison3.5.3 > mussurana 3.5.3 > tadarida 3.5.3 > urocryon 3.5.3 > > Presumably that only tells you about the animals currently building with python. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
Re: Estimating HugePages Requirements?
On Fri, Apr 22, 2022 at 09:49:34AM +0200, Magnus Hagander wrote: > I agree that thats a very narrow use case. And I'm not sure the use case of > a running server is even that important here - it's really the offline one > that's important. Or rather, the really compelling one is when there is a > server running but I want to check the value offline because it will > change. SHOW doesn't help there because it shows the value based on the > currently running configuration, not the new one after a restart. You mean the case of a server where one would directly change postgresql.conf on a running server, and use postgres -C to see how much the kernel settings need to be changed before the restart? > Hmm. So what's the solution on windows? I guess maybe it's not as important > there because there is no limit on huge pages, but generally getting the > expected shared memory usage might be useful? Just significantly less > important. Contrary to Linux, we don't need to care about the number of large pages that are necessary because there is no equivalent of vm.nr_hugepages on Windows (see [1]), do we? If that were the case, we'd have a use case for huge_page_size, additionally. That's the case where shared_memory_size_in_huge_pages comes in handy, as much as does huge_page_size, and note that shared_memory_size works on WIN32. [1]: https://docs.microsoft.com/en-us/windows/win32/memory/large-page-support -- Michael signature.asc Description: PGP signature
Re: Cryptohash OpenSSL error queue in FIPS enabled builds
On Sat, Apr 23, 2022 at 11:40:19PM +0200, Daniel Gustafsson wrote: > On 22 Apr 2022, at 19:01, Tom Lane wrote: >> Daniel Gustafsson writes: >>> Consuming all (both) errors and creating a concatenated string seems >>> overkill >>> as it would alter the API from a const error string to something that needs >>> freeing etc (also, very few OpenSSL consumers actually drain the queue, >>> OpenSSL >>> themselves don't). Skimming the OpenSSL code I was unable to find another >>> example of two errors generated. The attached calls ERR_clear_error() as >>> how >>> we do in libpq in order to avoid consuming earlier errors. It looks like the initialization error would come only from evp_md_init_internal() in digest.c. >> This seems quite messy. How would clearing the queue *before* creating >> the object improve matters? > > We know there won't be any leftovers which would make us display the wrong > message. Yeah. >> It seems like that solution means you're leaving an extra error in the queue >> to >> break unrelated code. Wouldn't it be better to clear after grabbing the >> error? >> (Or maybe do both.) > > That's a very good point, doing it in both ends of the operation is better > here. Error queues are cleaned with ERR_clear_error() before specific SSL calls in the frontend and the backend, never after the fact. If we assume that multiple errors can be stacked in the OpenSSL error queue, shouldn't we worry about cleaning up the error queue in code paths like pgtls_read/write(), be_tls_read/write() and be_tls_open_server()? So it seems to me that SSLerrmessage() should be treated the same way for the backend and the frontend. Any opinions? pgcrypto's openssl.c has the same problem under FIPS as it includes EVP calls. Saying that, putting a cleanup in pg_cryptohash_create() before the fact, and one in SSLerrmessage() after consuming the error code should be fine to keep a clean queue. Daniel, were you planning to write a patch? The other parts of the code are older than the hmac and cryptohash business, but I would not mind writing something for the whole. -- Michael signature.asc Description: PGP signature
Re: pg_receivewal fail to streams when the partial file to write is not fully initialized present in the wal receiver directory
On Fri, Apr 22, 2022 at 07:17:37PM +0530, Bharath Rupireddy wrote: > Right. We find enough disk space and go to write and suddenly the > write operations fail for some reason or the VM crashes because of a > reason other than disk space. I think the foolproof solution is to > figure out the available disk space before prepadding or compressing > and also use the > write-first-to-temp-file-and-then-rename-it-to-original-file as > proposed in the earlier patches in this thread. Yes, what would count here is only the amount of free space in a partition. The total amount of space available becomes handy once you begin introducing things like percentage-based quota policies for the disk when archiving. The free amount of space could be used to define a policy based on the maximum number of bytes you need to leave around, as well, but this is not perfect science as this depends of what FSes decide to do underneath. There are a couple of designs possible here. When I had to deal with my upthread case I have chosen one as I had no need to worry only about Linux, it does not mean that this is the best choice that would fit with the long-term community picture. This comes down to how much pg_receivewal should handle automatically, and how it should handle it. > Having said that, is there a portable way that we can find out the > disk space available? I read your response upthread that statvfs isn't > portable to WIN32 platforms. So, we just say that part of the fix we > proposed here (checking disk space before prepadding or compressing) > isn't supported on WIN32 and we just do the temp file thing for WIN32 > alone? Something like GetDiskFreeSpaceA() would do the trick on WIN32. https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-getdiskfreespacea When it comes to compression, creating a temporary file would only lead to its deletion, mostly, and extra I/O is never free. Anyway, why would you need the extra logic of the temporary file at all? That's basically what the .partial file is as pg_receivewal begins streaming at the beginning of a segment, to the partial file, each time it sees fit to restart a streaming cycle. -- Michael signature.asc Description: PGP signature
Re: [PATCH] Add native windows on arm64 support
On Thu, Apr 21, 2022 at 10:21:04AM +0100, Niyas Sait wrote: > The following error occurs: > > LINK : fatal error LNK1246: '/DYNAMICBASE:NO' not compatible with 'ARM64' > target machine; link without '/DYNAMICBASE:NO Okay, that's interesting. In light of things like 7f3e17b, that may be annoying. Perhaps newer Windows versions are able to handle that better. I am wondering if it would be worth re-evaluating this change, and attempt to re-enable that in environments other than arm64. This could become interesting if we consider that there have been talks to cut the support for a bunch of Windows versions to focus on the newer ones only. > This seems to be a deliberate restriction for Arm64 targets. However, no > references were provided. To clarify, I have posted a question [1] on the > community channel of Visual Studio. Thanks for doing that! Your post is just a couple of days old, let's see if we get a reply on that. -- Michael signature.asc Description: PGP signature
RE: Data is copied twice when specifying both child and parent table in publication
On Sun, Apr 24, 2022 at 2:16 PM I wrote: > On Thur, Apr 21, 2022 at 5:41 PM Amit Kapila wrote: > > IIRC, the column list and row filter also have some issues exactly due to > > this > > reason, so, I would like those cases to be also mentioned here and probably > > include the tests for them in the patch for HEAD. > Improve the test case about the column list and row filter to cover this bug. Sorry, I forgot to explain why I modify the tests for row filter and column filter. If we specify different filters on the parent and child table respectively, this bug will make us use the wrong filter. Like the following cases: [row filter] - environment in publisher-side. create table t (a int) partition by range (a); create table t_1 partition of t default; create publication pub1 for table t where (a<=10) with (PUBLISH_VIA_PARTITION_ROOT=true); create publication pub2 for table t_1 where (a>10) with (PUBLISH_VIA_PARTITION_ROOT=true); insert into t values (9),(11); - environment in subscriber-side. create table t (a int) partition by range (a); create table t_1 partition of t default; create subscription sub connection 'dbname=postgres user=postgres' publication pub1,pub2; When we execute the following SQL in subscriber-side, what we expect should be: select * from t; a --- 9 (1 row) but the HEAD is: a 9 11 (2 rows) [column filter] - environment in publisher-side. create table t (a int primary key, b int, c int) partition by range (a); create table t_1 partition of t default; create publication pub1 for table t(a, b) with (PUBLISH_VIA_PARTITION_ROOT=true); create publication pub2 for table t_1(a, c) with (PUBLISH_VIA_PARTITION_ROOT=true); insert into t values (1,1,1); - environment in subscriber-side. create table t (a int, b int, c int) partition by range (a); create table t_1 partition of t default; create subscription sub connection 'dbname=postgres user=postgres' publication pub1,pub2; When we execute the following SQL in subscriber-side, what we expect should be: select * from t; a | b | c ---+---+--- 1 | 1 | (1 row) but the HEAD is: a | b | c ---+---+--- 1 | 1 | 1 | | 1 (2 rows) Regards, Wang wei
Re: [Proposal] vacuumdb --schema only
On Fri, Apr 22, 2022 at 10:57:46PM -0700, Nathan Bossart wrote: > On Fri, Apr 22, 2022 at 11:57:05AM +0200, Gilles Darold wrote: >> Patch v10 attached. > > Thanks! I've attached a v11 with some minor editorialization. I think I > was able to improve the error handling for invalid combinations of > command-line options a bit, but please let me know what you think. I've attached a v12 of the patch that further simplifieѕ check_objfilter(). Apologies for the noise. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com >From b920a5d32d67d45b6cf25879c7bac81393612e59 Mon Sep 17 00:00:00 2001 From: Nathan Bossart Date: Fri, 22 Apr 2022 22:34:38 -0700 Subject: [PATCH v12 1/1] Add --schema and --exclude-schema options to vacuumdb. These two new options can be used to either process all tables in specific schemas or to skip processing all tables in specific schemas. This change also refactors the handling of invalid combinations of command-line options to a new helper function. Author: Gilles Darold Reviewed-by: Justin Pryzby, Nathan Bossart Discussion: https://postgr.es/m/929fbf3c-24b8-d454-811f-1d5898ab3e91%40migops.com --- doc/src/sgml/ref/vacuumdb.sgml| 66 ++ src/bin/scripts/t/100_vacuumdb.pl | 35 ++ src/bin/scripts/t/101_vacuumdb_all.pl | 3 + src/bin/scripts/vacuumdb.c| 168 +++--- 4 files changed, 231 insertions(+), 41 deletions(-) diff --git a/doc/src/sgml/ref/vacuumdb.sgml b/doc/src/sgml/ref/vacuumdb.sgml index 956c0f01cb..841aced3bd 100644 --- a/doc/src/sgml/ref/vacuumdb.sgml +++ b/doc/src/sgml/ref/vacuumdb.sgml @@ -39,6 +39,40 @@ PostgreSQL documentation dbname + + vacuumdb + connection-option + option + + + + + + + + -n + --schema + + schema + + + + + + + -N + --exclude-schema + + schema + + + + + + + dbname + + vacuumdb connection-option @@ -244,6 +278,30 @@ PostgreSQL documentation + + -n schema + --schema=schema + + +Clean or analyze all tables in +schema only. Multiple +schemas can be vacuumed by writing multiple -n switches. + + + + + + -N schema + --exclude-schema=schema + + +Do not clean or analyze any tables in +schema. Multiple schemas +can be excluded by writing multiple -N switches. + + + + --no-index-cleanup @@ -619,6 +677,14 @@ PostgreSQL documentation $ vacuumdb --analyze --verbose --table='foo(bar)' xyzzy + +To clean all tables in the foo and bar schemas +in a database named xyzzy: + +$ vacuumdb --schema='foo' --schema='bar' xyzzy + + + diff --git a/src/bin/scripts/t/100_vacuumdb.pl b/src/bin/scripts/t/100_vacuumdb.pl index 96a818a3c1..e4aac53249 100644 --- a/src/bin/scripts/t/100_vacuumdb.pl +++ b/src/bin/scripts/t/100_vacuumdb.pl @@ -103,6 +103,8 @@ $node->safe_psql( CREATE TABLE funcidx (x int); INSERT INTO funcidx VALUES (0),(1),(2),(3); CREATE INDEX i0 ON funcidx ((f1(x))); + CREATE SCHEMA "Foo"; + CREATE TABLE "Foo".bar(id int); |); $node->command_ok([qw|vacuumdb -Z --table="need""q(uot"(")x") postgres|], 'column list'); @@ -146,5 +148,38 @@ $node->issues_sql_like( [ 'vacuumdb', '--min-xid-age', '2147483001', 'postgres' ], qr/GREATEST.*relfrozenxid.*2147483001/, 'vacuumdb --table --min-xid-age'); +$node->issues_sql_like( + [ 'vacuumdb', '--schema', '"Foo"', 'postgres' ], + qr/VACUUM "Foo".bar/, + 'vacuumdb --schema schema only'); +$node->issues_sql_like( + [ 'vacuumdb', '--exclude-schema', '"Foo"', 'postgres' ], + qr/(?:(?!VACUUM "Foo".bar).)*/, + 'vacuumdb --exclude-schema schema'); +$node->command_fails( + [ 'vacuumdb', '-n', 'pg_catalog', '-t', 'pg_class', 'postgres' ], + 'cannot use options -n and -t at the same time'); +$node->command_fails( + [ 'vacuumdb', '-n', 'pg_catalog', '-N', '"Foo"', 'postgres' ], + 'cannot use options -n and -N at the same time'); +$node->command_fails( + [ 'vacuumdb', '-a', '-N', '"Foo"' ], + 'cannot use options -a and -N at the same time'); +$node->command_fails( + [ 'vacuumdb', '-a', '-n', '"Foo"' ], + 'cannot use options -a and -n at the same time'); +$node->command_fails( + [ 'vacuumdb', '-a', '-t', '"Foo".bar' ], + 'cannot use options -a and -t at the same time'); +$node->command_fails( + [ 'vacuumdb', '-a', '-n', '"Foo"', 'postgres' ], + 'cannot use options -a and -n at the same time'); +$node->command_fails( + [ 'vacuumdb', '-a', '-d', 'postgres' ], + 'cannot use options -a and -d at the same time'); +$node->command_fails( + [ 'vacuumdb', '-a', 'postgres' ], + 'cannot use option -a and a dbname as argument at the same time'); + done_testing(); diff --git a/src/bin/scripts/t/101_vacuumdb_all.pl b/src/bin/scripts/t/101_vacuumdb_all.pl inde
Re: Defer selection of asynchronous subplans until the executor initialization stage
Hi, On Wed, Apr 20, 2022 at 2:04 AM Zhihong Yu wrote: > It is okay to keep the formation in your patch. I modified mark_async_capable_plan() a bit further; 1) adjusted code in the ProjectionPath case, just for consistency with other cases, and 2) tweaked/improved comments a bit. Attached is a new version of the patch (“prevent-async-2.patch”). As mentioned before, v14 has the same issue, so I created a fix for v14, which I’m attaching as well (“prevent-async-2-v14.patch”). In the fix I modified is_async_capable_path() the same way as mark_async_capable_plan() in HEAD, renaming it to is_async_capable_plan(), and updated some comments. Barring objections, I’ll push/back-patch these. Thanks! Best regards, Etsuro Fujita prevent-async-2.patch Description: Binary data prevent-async-2-v14.patch Description: Binary data
json_object returning jsonb reuslt different from returning json, returning text
select json_objectagg( k:v absent on null with unique keys returning text ) from ( values(1,1),(0, null),(3, null),(2,2),(4,null) ) foo(k, v); return json_objectagg -- { "1" : 1, "2" : 2 } select json_objectagg(k:v absent on null with unique keys) from ( values(1,1),(0, null),(3, null),(2,2),(4,null) ) foo(k, v); return json_objectagg -- { "1" : 1, "2" : 2 } *But* select json_objectagg( k:v absent on null with unique keys returning jsonb ) from ( values(1,1),(0, null),(3, null),(2,2),(4,null) ) foo(k, v); return json_objectagg - {"0": null, "1": 1, "2": 2} the last query "returning jsonb" should be { "1" : 1, "2" : 2 } ? version: > PostgreSQL 15devel (Ubuntu > 15~~devel~20220407.0430-1~713.git79b716c.pgdg20.04+1) on > x86_64-pc-linux-gnu, > compiled by gcc (Ubuntu 9.4.0-1ubuntu1~20.04.1) 9.4.0, 64-bit
Re: json_object returning jsonb reuslt different from returning json, returning text
seems it's a bug around value 0. SELECT JSON_OBJECTAGG(k: v ABSENT ON NULL WITH UNIQUE KEYS RETURNING jsonb) FROM (VALUES (1, 1), (10, NULL),(4, null), (5, null),(6, null),(2, 2)) foo(k, v); return: {"1": 1, "2": 2} SELECT JSON_OBJECTAGG(k: v ABSENT ON NULL WITH UNIQUE KEYS RETURNING jsonb) FROM (VALUES (1, 1), (0, NULL),(4, null), (5, null),(6, null),(2, 2)) foo(k, v); return {"0": null, "1": 1, "2": 2} On Mon, Apr 25, 2022 at 10:41 AM alias wrote: > select json_objectagg( > k:v absent on null with unique keys returning text ) > from ( > values(1,1),(0, null),(3, null),(2,2),(4,null) > ) foo(k, v); > > return > > json_objectagg > -- > { "1" : 1, "2" : 2 } > > > select json_objectagg(k:v absent on null with unique keys) > from ( > values(1,1),(0, null),(3, null),(2,2),(4,null) > ) foo(k, v); > > return > > json_objectagg -- { "1" : 1, "2" : 2 } > > *But* > > select json_objectagg( > k:v absent on null with unique keys returning jsonb ) > from ( > values(1,1),(0, null),(3, null),(2,2),(4,null) > ) foo(k, v); > > return > json_objectagg - {"0": null, "1": 1, "2": 2} > > the last query "returning jsonb" should be { "1" : 1, "2" : 2 } ? > > version: > >> PostgreSQL 15devel (Ubuntu >> 15~~devel~20220407.0430-1~713.git79b716c.pgdg20.04+1) on >> x86_64-pc-linux-gnu, >> compiled by gcc (Ubuntu 9.4.0-1ubuntu1~20.04.1) 9.4.0, 64-bit > >
Re: [PATCH] Teach pg_waldump to extract FPIs from the WAL
On Sat, Apr 23, 2022 at 01:43:36PM -0500, David Christensen wrote: > Hi Matthias, great point. Enclosed is a revised version of the patch > that adds the fork identifier to the end if it's a non-main fork. Like Alvaro, I have seen cases where this would have been really handy. So +1 from me, as well, to have more tooling like what you are proposing. Fine for me to use one file for each block with a name like what you are suggesting for each one of them. + /* we accept an empty existing directory */ + if (stat(config.save_fpw_path, &st) == 0 && S_ISDIR(st.st_mode)) + { I don't think that there is any need to rely on a new logic if there is already some code in place able to do the same work. See verify_dir_is_empty_or_create() in pg_basebackup.c, as one example, that relies on pg_check_dir(). I think that you'd better rely at least on what pgcheckdir.c offers. + {"raw-fpi", required_argument, NULL, 'W'}, I think that we'd better rename this option. "fpi", that is not used much in the user-facing docs, is additionally not adapted when we have an other option called -w/--fullpage. I can think of --save-fullpage. + PageSetLSN(page, record->ReadRecPtr); + /* if checksum field is non-zero then we have checksums enabled, +* so recalculate the checksum with new LSN (yes, this is a hack) +*/ Yeah, that looks like a hack, but putting in place a page on a cluster that has checksums enabled would be more annoying with zero_damaged_pages enabled if we don't do that, so that's fine by me as-is. Perhaps you should mention that FPWs don't have their pd_checksum updated when written. + /* we will now extract the fullpage image from the XLogRecord and save +* it to a calculated filename */ The format of this comment is incorrect. +The LSN of the record with this block, formatted +as %08x-%08X instead of the +conventional %X/%X due to filesystem naming +limits The last part of the sentence about %X/%X could just be removed. That could be confusing, at worse. + PageSetLSN(page, record->ReadRecPtr); Why is pd_lsn set? git diff --check complains a bit. This stuff should include some tests. With --end, the tests can be cheap. -- Michael signature.asc Description: PGP signature
Re: why pg_walfile_name() cannot be executed during recovery?
> 22 апр. 2022 г., в 19:15, Bharath Rupireddy > написал(а): > > On Sat, Apr 9, 2022 at 10:21 PM Robert Haas wrote: >> >> On Sat, Apr 9, 2022 at 12:25 PM Andrey Borodin wrote: >>> Please excuse me if I'm not attentive enough. I've read this thread. And I >>> could not find what is the problem that you are solving. What is the >>> purpose of the WAL file name you want to obtain? >> >> Yeah, I'd also like to know this. > > IMO, uses of pg_walfile_{name, name_offset} are plenty. Say, I have > LSNs (say, flush, insert, replayed or WAL receiver latest received) AFAIK flush, receive and replay LSNs may be on 3 different timelines rendering two names incorrect. Actually, this proves that pg_wal_filename() should not be called on Standby with a present function prototype. > and I would like to know the WAL file name and offset in an app > connecting to postgres or a control plane either for doing some > reporting What kind of reporting? > or figuring out whether a WAL file exists given an LSN or > for some other reason. There might me many WAL files on the same LSN. Please, specify more detailed scenario to use WAL file name. > With these functions restricted when the server > is in recovery mode, the apps or control plane code can't use them and > they have to do if (!pg_is_in_recovery()) {select * from > pg_walfile_{name, name_offset}. > > Am I missing any other important use-cases? I do not see correct use-case among these. You justify necessity to run pg_wal_filename() on Standby by having a LSN (not a problem), by doing some kind of reporting (to broad problem) and checking existence of some WAL file (more details needed). What is the problem leading to checking the existence of the file? Thanks! Best regards, Andrey Borodin.
Re: TRAP: FailedAssertion("tabstat->trans == trans", File: "pgstat_relation.c", Line: 508
On Wed, Apr 20, 2022 at 01:03:20PM +0200, Erik Rijkers wrote: > Yes, that seems to fix it: I applied that latter patch, and ran my program > 250x without errors. Then I removed it again an it gave the error within > 15x. That looks simple enough, indeed. Andres, are you planning to address this issue? -- Michael signature.asc Description: PGP signature
Re: [Proposal] vacuumdb --schema only
goo Le 25/04/2022 à 03:27, Nathan Bossart a écrit : > On Fri, Apr 22, 2022 at 10:57:46PM -0700, Nathan Bossart wrote: >> On Fri, Apr 22, 2022 at 11:57:05AM +0200, Gilles Darold wrote: >>> Patch v10 attached. >> Thanks! I've attached a v11 with some minor editorialization. I think I >> was able to improve the error handling for invalid combinations of >> command-line options a bit, but please let me know what you think. > I've attached a v12 of the patch that further simplifieѕ check_objfilter(). > Apologies for the noise. > Looks good for me, there is a failure on cfbot on FreeBSD but I have run a CI with latest master and it pass. Can I change the commitfest status to ready for committers? -- Gilles Darold