Re: Fix japanese translation of log messages
On 2022-08-26 14:07, Kyotaro Horiguchi wrote: At Fri, 26 Aug 2022 10:23:01 +0900, Shinya Kato wrote in I've found typos in ja.po, and fixed them. The patch is attached. (This is not for -hackers but I'm fine with it being posted here;p) Sorry, I didn't know there was an pgsql-translators. Thanks for the report! Pushed to 10 to 15 of translation repository with some minor changes. They will be reflected in the code tree some time later. Thanks! msgid "More details may be available in the server log." -msgstr "詳細な情報がはサーバログにあるかもしれません。" +msgstr "詳細な情報はサーバログにあるかもしれません。" I prefer "詳細な情報が" than "詳細な情報は" here. (The existnce of the details is unknown here.) msgid "cannot drop active portal \"%s\"" -msgstr "アクテイブなポータル\"%s\"を削除できません" +msgstr "アクティブなポータル\"%s\"を削除できません" I canged it to "アクティブなポータル\"%s\"は削除できません". (It describes state facts, not telling the result of an action.) Thanks, LGTM. -- Regards, -- Shinya Kato Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION
Re: [PATCH] Optimize json_lex_string by batching character copying
On Thu, Aug 25, 2022 at 1:35 PM John Naylor wrote: > > I think I'll go ahead and commit 0001 in a couple days pending further > comments. Pushed with Nathan's correction and some cosmetic rearrangements. -- John Naylor EDB: http://www.enterprisedb.com
Re: pg_rewind WAL segments deletion pitfall
(Moved to -hackers) At Thu, 25 Aug 2022 10:34:40 +0200, Alexander Kukushkin wrote in > > # killall -9 postgres > > # rm -r oldprim newprim oldarch newarch oldprim.log newprim.log > > mkdir newarch oldarch > > initdb -k -D oldprim > > echo "archive_mode = 'always'">> oldprim/postgresql.conf > > > > With archive_mode = always you can't reproduce it. > It is very rarely people set it to always in production due to the overhead. ... > The archive_mode has to be set to on and the archive_command should be > failing when you do pg_ctl -D oldprim stop Ah, I see. What I don't still understand is why pg_rewind doesn't work for the old primary in that case. When archive_mode=on, the old primary has the complete set of WAL files counting both pg_wal and its archive. So as the same to the privious repro, pg_rewind -c ought to work (but it uses its own archive this time). In that sense the proposed solution is still not needed in this case. A bit harder situation comes after the server successfully rewound; if the new primary goes so far that the old primary cannot connect. Even in that case, you can copy-in the requried WAL files or configure restore_command of the old pimary so that it finds required WAL files there. As the result the system in total doesn't lose a WAL file. So.. I might still be missing something.. ### # killall -9 postgres # rm -r oldprim newprim oldarch newarch oldprim.log newprim.log mkdir newarch oldarch initdb -k -D oldprim echo "archive_mode = 'on'">> oldprim/postgresql.conf echo "archive_command = 'cp %p `pwd`/oldarch/%f'">> oldprim/postgresql.conf pg_ctl -D oldprim -o '-p 5432' -l oldprim.log start psql -p 5432 -c 'create table t(a int)' pg_basebackup -D newprim -p 5432 echo "primary_conninfo='host=/tmp port=5432'">> newprim/postgresql.conf echo "archive_command = 'cp %p `pwd`/newarch/%f'">> newprim/postgresql.conf touch newprim/standby.signal pg_ctl -D newprim -o '-p 5433' -l newprim.log start # the last common checkpoint psql -p 5432 -c 'checkpoint' # record approx. diverging WAL segment start_wal=`psql -p 5433 -Atc 'select pg_walfile_name(pg_last_wal_replay_lsn() - (select setting from pg_settings where name = 'wal_segment_size')::int); ` psql -p 5432 -c 'insert into t values(0); select pg_switch_wal();' pg_ctl -D newprim promote psql -p 5433 -c 'checkpoint' # old rprimary loses diverging WAL segment for i in $(seq 1 4); do psql -p 5432 -c 'insert into t values(0); select pg_switch_wal();'; done # old primary cannot archive any more echo "archive_command = 'false'">> oldprim/postgresql.conf pg_ctl -D oldprim reload pg_ctl -D oldprim stop # rewind the old primary, using its own archive # pg_rewind -D oldprim --source-server='port=5433' # should fail echo "restore_command = 'cp `pwd`/oldarch/%f %p'">> oldprim/postgresql.conf pg_rewind -D oldprim --source-server='port=5433' -c # advance WAL on the old primary; new primary loses the launching WAL seg for i in $(seq 1 4); do psql -p 5433 -c 'insert into t values(0); select pg_switch_wal();'; done psql -p 5433 -c 'checkpoint' echo "primary_conninfo='host=/tmp port=5433'">> oldprim/postgresql.conf touch oldprim/standby.signal postgres -D oldprim # fails with "WAL file has been removed" # The alternative of copying-in # echo "restore_command = 'cp `pwd`/newarch/%f %p'">> oldprim/postgresql.conf # copy-in WAL files from new primary's archive to old primary (cd newarch; for f in `ls`; do if [[ "$f" > "$start_wal" ]]; then echo copy $f; cp $f ../oldprim/pg_wal; fi done) postgres -D oldprim -- Kyotaro Horiguchi NTT Open Source Software Center
Re: [PATCH] Fix alter subscription concurrency errors
> Won't the same thing can happen for similar publication commands? Why > is this unique to the subscription and not other Alter/Drop commands? I indeed don't think this problem is unique to subscriptions, but it seems better to at least have this problem in a few places less (not making perfect the enemy of good). If someone has a more generic way of solving this for other commands too, then that sounds great, but if not then slowly chipping away at these cases seems better than keeping the status quo. Attached is a new patch where ALTER SUBSCRIPTION ... OWNER TO ... can now also be executed concurrently with the other subscription commands. 0001-Fix-errors-when-concurrently-altering-subscriptions.patch Description: 0001-Fix-errors-when-concurrently-altering-subscriptions.patch
Re: Asynchronous execution support for Custom Scan
Hi KaiGai-san, On Tue, Aug 23, 2022 at 6:26 PM Kohei KaiGai wrote: > I internally suggested him to expand the ctidscan module for the PoC purpose. > https://github.com/kaigai/ctidscan > > Even though it does not have asynchronous capability actually, but > suitable to ensure > API works and small enough for reviewing. Seems like a good idea. Thanks! Best regards, Etsuro Fujita
Re: Fix japanese translation of log messages
On 2022-Aug-26, Kyotaro Horiguchi wrote: > It's a mistake of "には". I'll load it into the next ship. The next > release is 9/8 and I'm not sure the limit of translation commits for > the release, though.. Typically the translations are updated from the pgtranslation repository on Monday of the release week, at around noon European time. You can keep translating till the previous Sunday if you feel like it :-) -- Álvaro HerreraBreisgau, Deutschland — https://www.EnterpriseDB.com/
[PATCH] Add peer authentication TAP test
Hi hackers,
During the work in [1] we created a new TAP test to test the SYSTEM_USER
behavior with peer authentication.
It turns out that there is currently no TAP test for the peer
authentication, so we think (thanks Michael for the suggestion [2]) that
it's better to split the work in [1] between "pure" SYSTEM_USER related
work and the "pure" peer authentication TAP test work.
That's the reason of this new thread, please find attached a patch to
add a new TAP test for the peer authentication.
[1]:
https://www.postgresql.org/message-id/flat/7e692b8c-0b11-45db-1cad-3afc5b57409f%40amazon.com
[2]: https://www.postgresql.org/message-id/YwgboqQUV1%2BY/k6z%40paquier.xyz
Regards,
--
Bertrand Drouvot
Amazon Web Services: https://aws.amazon.com
diff --git a/src/test/authentication/t/003_peer.pl
b/src/test/authentication/t/003_peer.pl
new file mode 100644
index 00..006c4405a5
--- /dev/null
+++ b/src/test/authentication/t/003_peer.pl
@@ -0,0 +1,140 @@
+
+# Copyright (c) 2022, PostgreSQL Global Development Group
+
+# Tests for peer authentication and user name map.
+# The test is skipped if the platform does not support peer authentication.
+
+use strict;
+use warnings;
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+# Delete pg_hba.conf from the given node, add a new entry to it
+# and then execute a reload to refresh it.
+sub reset_pg_hba
+{
+ my $node = shift;
+ my $hba_method = shift;
+
+ unlink($node->data_dir . '/pg_hba.conf');
+ # just for testing purposes, use a continuation line.
+ $node->append_conf('pg_hba.conf', "local all all\\\n $hba_method");
+ $node->reload;
+ return;
+}
+
+# Test access for a single role, useful to wrap all tests into one.
+sub test_role
+{
+ local $Test::Builder::Level = $Test::Builder::Level + 1;
+
+ my ($node, $role, $method, $expected_res) = @_;
+ my $status_string = 'failed';
+ $status_string = 'success' if ($expected_res eq 0);
+
+ my $connstr = "user=$role";
+ my $testname =
+"authentication $status_string for method $method, role $role";
+
+ if ($expected_res eq 0)
+ {
+$node->connect_ok($connstr, $testname);
+ }
+ else
+ {
+ # No checks of the error message, only the status code.
+ $node->connect_fails($connstr, $testname);
+ }
+}
+
+# return the size of logfile of $node in bytes.
+sub get_log_size
+{
+ my ($node) = @_;
+
+ return (stat $node->logfile)[7];
+}
+
+# find $pat in logfile of $node after $off-th byte.
+sub find_in_log
+{
+ my ($node, $pat, $off) = @_;
+
+ $off = 0 unless defined $off;
+ my $log = PostgreSQL::Test::Utils::slurp_file($node->logfile);
+ return 0 if (length($log) <= $off);
+
+ $log = substr($log, $off);
+
+ return $log =~ m/$pat/;
+}
+
+# Initialize primary node.
+my $node = PostgreSQL::Test::Cluster->new('primary');
+$node->init;
+$node->append_conf('postgresql.conf', "log_connections = on\n");
+$node->start;
+
+# Get the session_user to define the user name map test.
+my $session_user =
+ $node->safe_psql('postgres', 'select session_user');
+
+# Create a new user for the user name map test.
+$node->safe_psql('postgres',
+ qq{CREATE USER testmap$session_user});
+
+# Set pg_hba.conf with the peer authentication.
+reset_pg_hba($node, 'peer');
+
+# Check that peer authentication is supported on this platform.
+my $logstart = get_log_size($node);
+
+$node->psql('postgres', undef, connstr => "user=$session_user");
+
+# If not supported, then skip the rest of the test.
+if (find_in_log($node, qr/peer authentication is not supported on this
platform/, $logstart))
+{
+plan skip_all => 'peer authentication is not supported on this platform';
+}
+
+# It's supported so let's test peer authentication.
+# This connection should succeed.
+$logstart = get_log_size($node);
+test_role($node, $session_user, 'peer', 0);
+
+ok( find_in_log(
+ $node,
+ qr/connection authenticated: identity="$session_user" method=peer/,
$logstart),
+ "logfile: connection authenticated for method peer and identity
$session_user");
+
+# This connection should failed.
+$logstart = get_log_size($node);
+test_role($node, qq{testmap$session_user}, 'peer', 2);
+
+ok( find_in_log(
+ $node,
+ qr/Peer authentication failed for user "testmap$session_user"/,
$logstart),
+ "logfile: Peer authentication failed for user testmap$session_user");
+
+# Define a user name map.
+$node->append_conf('pg_ident.conf', qq{mypeermap $session_user
testmap$session_user});
+
+# Set pg_hba.conf with the peer authentication and the user name map.
+reset_pg_hba($node, 'peer map=mypeermap');
+
+# This connection should now succeed.
+$logstart = get_log_size($node);
+test_role($node, qq{testmap$session_user}, 'peer', 0);
+
+ok( find_in_log(
+ $node,
+ qr/connection authenticated: identity="$session_user" method=peer/,
$logstart),
+ "logfile: connection authenticated for method peer and identity
$session_user");
+
+ok( f
Re: Fix japanese translation of log messages
At Fri, 26 Aug 2022 10:25:17 +0200, Alvaro Herrera wrote in > Typically the translations are updated from the pgtranslation repository > on Monday of the release week, at around noon European time. You can > keep translating till the previous Sunday if you feel like it :-) Yeah... . . So.. the limit is around 9/5 12:00 CEST(?).. is.. 9/5 19:00 JST? Thank you very much for the significant info. regards. -- Kyotaro Horiguchi NTT Open Source Software Center
Re: pg_receivewal and SIGTERM
Re: Daniel Gustafsson > The attached adds the Exit Status section to pg_recvlogical docs which is > present in pg_receivewal to make them more aligned, and tweaks comments to > pgindent standards. This is the version I think is ready to commit. Looks good to me. Thanks, Christoph
Re: [PoC] Reducing planning time when tables have many partitions
Dear David, On Fri, Aug 26, 2022 at 12:18 PM David Rowley wrote: > How about instead of doing this caching like this, why don't we code > up some iterators that we can loop over to fetch the required EMs. Thank you very much for your quick reply and for sharing your idea with code. I also think introducing EquivalenceMemberIterator is one good alternative solution. I will try to implement and test it. Thank you again for helping me. -- Best regards, Yuya Watari
Re: pg_rewind WAL segments deletion pitfall
Hello Kyotaro, On Fri, 26 Aug 2022 at 10:04, Kyotaro Horiguchi wrote: > > With archive_mode = always you can't reproduce it. > > It is very rarely people set it to always in production due to the > overhead. > ... > > The archive_mode has to be set to on and the archive_command should be > > failing when you do pg_ctl -D oldprim stop > > Ah, I see. > > What I don't still understand is why pg_rewind doesn't work for the > old primary in that case. When archive_mode=on, the old primary has > the complete set of WAL files counting both pg_wal and its archive. So > as the same to the privious repro, pg_rewind -c ought to work (but it > uses its own archive this time). In that sense the proposed solution > is still not needed in this case. > The pg_rewind finishes successfully. But as a result it removes some files from pg_wal that are required to perform recovery because they are missing on the new primary. > > A bit harder situation comes after the server successfully rewound; if > the new primary goes so far that the old primary cannot connect. Even > in that case, you can copy-in the requried WAL files or configure > restore_command of the old pimary so that it finds required WAL files > there. > Yes, we can do the backup of pg_wal before running pg_rewind, but it feels very ugly, because we will also have to clean this "backup" after a successful recovery. It would be much better if pg_rewind didn't remove WAL files between the last common checkpoint and diverged LSN in the first place. Regards, -- Alexander Kukushkin
Re: use SSE2 for is_valid_ascii
On Fri, Aug 26, 2022 at 10:26 AM Nathan Bossart wrote: > > On Thu, Aug 25, 2022 at 04:41:53PM +0700, John Naylor wrote: > > v3 applies on top of the v9 json_lex_string patch in [1] and adds a > > bit more to that, resulting in a simpler patch that is more amenable > > to additional SIMD-capable platforms. > > LGTM Thanks for looking, pushed with some rearrangements. -- John Naylor EDB: http://www.enterprisedb.com
Re: Fix japanese translation of log messages
On 2022-Aug-26, Kyotaro Horiguchi wrote: > At Fri, 26 Aug 2022 10:25:17 +0200, Alvaro Herrera > wrote in > > Typically the translations are updated from the pgtranslation repository > > on Monday of the release week, at around noon European time. You can > > keep translating till the previous Sunday if you feel like it :-) > > Yeah... . . > > So.. the limit is around 9/5 12:00 CEST(?).. is.. 9/5 19:00 JST? Well, Sept 8th is the date of 15 beta4. I suppose there'll be at least two or three weeks from beta4 to the RC1, and maybe one or two more weeks from there to 15.0. You can obviously continue to translate until then, if you want these translations to appear in 15.0. And as for stable branches, the next one is scheduled for early November, so you have until then to fix typos in those. For any translations that do not appear in 15.0, you have three more months until 15.1 ... and so on. It never ends. Blessing or curse? -- Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/ Officer Krupke, what are we to do? Gee, officer Krupke, Krup you! (West Side Story, "Gee, Officer Krupke")
Re: making relfilenodes 56 bits
On Thu, Aug 25, 2022 at 5:26 PM Dilip Kumar wrote: > I agree on this that this system is easy to explain that we just > rewrite anything that conflicts so looks more future-proof. Okay, I > will try this solution and post the patch. While working on this solution I noticed one issue. Basically, the problem is that during binary upgrade when we try to rewrite a heap we would expect that “binary_upgrade_next_heap_pg_class_oid” and “binary_upgrade_next_heap_pg_class_relfilenumber” are already set for creating a new heap. But we are not preserving anything so we don't have those values. One option to this problem is that we can first start the postmaster in non-binary upgrade mode perform all conflict checking and rewrite and stop the postmaster. Then start postmaster again and perform the restore as we are doing now. Although we will have to start/stop the postmaster one extra time we have a solution. But while thinking about this I started to think that since now we are completely decoupling the concept of Oid and relfilenumber then logically during REWRITE we should only be allocating new relfilenumber but we don’t really need to allocate the Oid at all. Yeah, we can do that if inside make_new_heap() if we pass the OIDOldHeap to heap_create_with_catalog(), then it will just create new storage(relfilenumber) but not a new Oid. But the problem is that the ATRewriteTable() and finish_heap_swap() functions are completely based on the relation cache. So now if we only create a new relfilenumber but not a new Oid then we will have to change this infrastructure to copy at smgr level. Thoughts? -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com
Re: Insertion Sort Improvements
On Thu, Aug 25, 2022 at 5:55 PM Benjamin Coutu wrote:
>
> Hello,
>
> Inspired by the recent discussions[1][2] around sort improvements, I took a
> look around the code and noticed the use of a somewhat naive version of
> insertion sort within the broader quicksort code.
>
> The current implementation (see sort_template.h) is practically the textbook
> version of insertion sort:
Right. I think it's worth looking into. When I tested dual-pivot
quicksort, a threshold of 18 seemed best for my inputs, so making
insertion sort more efficient could tilt the balance more in favor of
dual-pivot. (It already seems slightly ahead, but as I mentioned in
the thread you linked, removing branches for null handling would make
it more compelling).
> DO_ASSIGN and DO_COPY macros would have to be declared analogue to DO_SWAP
> via the template.
I don't think you need these macros, since this optimization is only
convenient if you know the type at compile time. See the attached,
which I had laying around when I was looking at PDQuicksort. I believe
it's similar to what you have in mind. (Ignore the part about
"unguarded", it's irrelevant out of context). Notice that it avoids
unnecessary copies, but has two calls to DO_COMPARE, which is *not*
small for tuples.
> Anyways, there might be some low hanging fruit here. If it turns out to be
> significantly faster one might even consider increasing the insertion sort
> threshold from < 7 to < 10 sort elements. But that is a whole other
> discussion for another day.
I think it belongs around 10 now anyway. I also don't think you'll see
much benefit with your proposal at a threshold of 7 -- I suspect it'd
be more enlightening to test a range of thresholds with and without
the patch, to see how the inflection point shifts. That worked pretty
well when testing dual-pivot.
--
John Naylor
EDB: http://www.enterprisedb.com
From fd4bfdbee88478fac32838712c112d91f73c5db5 Mon Sep 17 00:00:00 2001
From: John Naylor
Date: Wed, 4 May 2022 23:35:06 +0700
Subject: [PATCH v3 8/8] Use unguarded insertion sort where possible
Since we recurse to the partitions in order, guarding is only
needed at the very beginning of the input. It's not yet clear
if this is a performance win for sort keys with complex
comparators.
---
src/include/lib/sort_template.h| 58 +-
src/test/regress/expected/geometry.out | 2 +-
2 files changed, 57 insertions(+), 3 deletions(-)
diff --git a/src/include/lib/sort_template.h b/src/include/lib/sort_template.h
index 3b3c4bc128..ae76fd39e8 100644
--- a/src/include/lib/sort_template.h
+++ b/src/include/lib/sort_template.h
@@ -79,7 +79,6 @@
* https://github.com/orlp/pdqsort
*
* WIP: Not yet implemented:
- * - use unguarded insertion sort where possible
* - fall back to heap sort to guard the stack
*
* Other techniques used in PDQuicksort, but likely not worth adopting:
@@ -174,6 +173,7 @@ ST_SCOPE void ST_SORT(ST_ELEMENT_TYPE * first, size_t n
/* sort private helper functions */
#define ST_INSERTION_SORT ST_MAKE_NAME(ST_SORT, insertion_sort)
#define ST_INSERTION_SORT_PARTIAL ST_MAKE_NAME(ST_SORT, insertion_sort_partial)
+#define ST_INSERTION_SORT_UNGUARDED ST_MAKE_NAME(ST_SORT, insertion_sort_unguarded)
#define ST_PARTITION_LEFT ST_MAKE_NAME(ST_SORT, partition_left)
#define ST_PARTITION_RIGHT ST_MAKE_NAME(ST_SORT, partition_right)
#define ST_SORT_INTERNAL ST_MAKE_NAME(ST_SORT, internal)
@@ -213,6 +213,12 @@ ST_SCOPE void ST_SORT(ST_ELEMENT_TYPE * first, size_t n
ST_SORT_INVOKE_COMPARE \
ST_SORT_INVOKE_ARG)
+#define DO_INSERTION_SORT_UNGUARDED(begin_, end_) \
+ ST_INSERTION_SORT_UNGUARDED((begin_), (end_) \
+ ST_SORT_INVOKE_ELEMENT_SIZE \
+ ST_SORT_INVOKE_COMPARE \
+ ST_SORT_INVOKE_ARG)
+
#define DO_PARTITION_LEFT(begin_, end_) \
ST_PARTITION_LEFT((begin_), (end_) \
ST_SORT_INVOKE_ELEMENT_SIZE \
@@ -402,6 +408,48 @@ ST_INSERTION_SORT_PARTIAL(ST_POINTER_TYPE * begin,
return true;
}
+static inline void
+ST_INSERTION_SORT_UNGUARDED(ST_POINTER_TYPE * begin,
+ ST_POINTER_TYPE * end
+ ST_SORT_PROTO_ELEMENT_SIZE
+ ST_SORT_PROTO_COMPARE
+ ST_SORT_PROTO_ARG)
+{
+ ST_POINTER_TYPE *cur;
+ ST_POINTER_TYPE *sift;
+ ST_POINTER_TYPE *sift_1;
+
+ if (begin == end)
+ return;
+
+ for (cur = begin + ST_POINTER_STEP; cur < end; cur += ST_POINTER_STEP)
+ {
+#ifndef ST_ELEMENT_TYPE_VOID
+ sift = cur;
+ sift_1 = cur - 1;
+
+ if (DO_COMPARE(sift_1, sift) > 0)
+ {
+ ST_ELEMENT_TYPE tmp;
+ tmp = *sift;
+
+ do
+ {
+*sift-- = *sift_1;
+sift_1--; /* Avoid multiple evaluation in DO_COMPARE */
+ } while (DO_COMPARE(sift_1, &tmp) > 0);
+
+ *sift = tmp;
+ }
+#else
+ for (sift = cur, sift_1 = cur - ST_POINTER_STEP;
+ DO_COMPARE(sift_1, sift) > 0;
+ sift -= ST_POINTER_STEP, sift_1 -= ST_POINTER_STEP)
+ DO_SWAP(sift, sift_1);
+#endif
+ }
+}
+
// Partitions [begin, end) around pivot *begin. Elements equal
//
Re: Bump MIN_WINNT to 0x0600 (Vista) as minimal runtime in 16~
On Thu, Jul 07, 2022 at 01:56:39PM +0900, Michael Paquier wrote: > On Thu, Jul 07, 2022 at 09:11:57AM +0900, Michael Paquier wrote: > > Okay, thanks for confirming. I think that I'll give it a try today > > then, my schedule would fit nicely with the buildfarm monitoring. > > And I have applied that, after noticing that the MinGW was complaining > because _WIN32_WINNT was not getting set like previously and removing > _WIN32_WINNT as there is no need for it anymore. The CI has reported > green for all my tests, so I am rather confident to not have messed up > something. Now, let's see what the buildfarm members tell. This > should take a couple of hours.. If I'm not wrong, there's some lingering comments which could be removed since 495ed0ef2. src/bin/pg_ctl/pg_ctl.c: * on NT4. That way, we don't break on NT4. src/bin/pg_ctl/pg_ctl.c: * On NT4, or any other system not containing the required functions, will src/bin/pg_ctl/pg_ctl.c: * NT4 doesn't have CreateRestrictedToken, so just call ordinary src/port/dirmod.c: *Win32 (NT4 and newer). src/backend/port/win32/socket.c:/* No error, zero bytes (win2000+) or error+WSAEWOULDBLOCK (<=nt4) */ -- Justin
windows cfbot failing: my_perl
The last 20 some consecutive builds failed: https://cirrus-ci.com/github/postgresql-cfbot/postgresql like this: [09:29:27.711] C:\Program Files (x86)\Windows Kits\10\Include\10.0.20348.0\ucrt\malloc.h(159,17): error C2065: 'my_perl': undeclared identifier (compiling source file src/pl/plperl/SPI.c) [c:\cirrus\plperl.vcxproj] [09:29:27.711] C:\Program Files (x86)\Windows Kits\10\Include\10.0.20348.0\ucrt\malloc.h(159,17): error C2065: 'my_perl': undeclared identifier (compiling source file src/pl/plperl/Util.c) [c:\cirrus\plperl.vcxproj] [09:29:27.711] C:\Program Files (x86)\Windows Kits\10\Include\10.0.20348.0\ucrt\malloc.h(159,17): error C2223: left of '->IMem' must point to struct/union (compiling source file src/pl/plperl/SPI.c) [c:\cirrus\plperl.vcxproj] [09:29:27.711] C:\Program Files (x86)\Windows Kits\10\Include\10.0.20348.0\ucrt\malloc.h(159,17): error C2223: left of '->IMem' must point to struct/union (compiling source file src/pl/plperl/Util.c) [c:\cirrus\plperl.vcxproj] I imagine it may be due to an error hit while rebuilding the ci's docker image. -- Justin
Re: wal_sync_method=fsync_writethrough
On Fri, Aug 26, 2022 at 6:55 AM Thomas Munro wrote: > > Hi, > > We allow $SUBJECT on Windows. I'm not sure exactly how we finished up > with that, maybe a historical mistake, but I find it misleading today. > Modern Windows flushes drive write caches for fsync (= _commit()) and > fdatasync (= FLUSH_FLAGS_FILE_DATA_SYNC_ONLY). In fact it is possible > to tell Windows to write out file data without flushing the drive > cache (= FLUSH_FLAGS_NO_SYNC), but I don't believe anyone is > interested in new weaker levels. Any reason not to just get rid of > it? So, I don't know how it works now, but the history at least was this: it was not about the disk caches, it was about raid controller caches. Basically, we determined that windows didn't fsync it all the way. But it would with But if we changed wal_sync_method=fsync to actually *do* that, then people who had paid big money for raid controllers with flash or battery backed cache would lose a ton of performance. So we needed one level that would sync out of the OS but not through the RAID cache, and another one that would sync it out of the RAID cache as well. Which would/could be different from the drive caches themselves, and they often behaved differently. And I think it may have even been dependent on the individual RAID drivers what the default would be. -- Magnus Hagander Me: https://www.hagander.net/ Work: https://www.redpill-linpro.com/
Re: Removing unneeded self joins
On 30/6/2022 17:11, Andrey Lepikhov wrote:
On 19/5/2022 16:47, Ronan Dunklau wrote:
I'll take a look at that one.
New version of the patch, rebased on current master:
1. pgindent over the patch have passed.
2. number of changed files is reduced.
3. Some documentation and comments is added.
New version rebased on new master, minor changes and tests added.
--
regards,
Andrey Lepikhov
Postgres ProfessionalFrom 8d864515da68728ddee10d455f8bdb64d34277aa Mon Sep 17 00:00:00 2001
From: "Andrey V. Lepikhov"
Date: Fri, 26 Aug 2022 15:17:53 +0300
Subject: [PATCH] Remove self-joins.
A Self Join Removal (SJR) feature removes inner join of plane table to itself
in a query plan if can be proved that the join can be replaced with a scan.
The proof based on innerrel_is_unique machinery.
We can remove a self-join when for each outer row:
1. At most one inner row matches the join clauses.
2. If the join target list contains any inner vars, an inner row
must be (physically) the same row as the outer one.
In this patch we use Rowley's [1] approach to identify a self-join:
1. Collect all mergejoinable join quals which look like a.x = b.x
2. Check innerrel_is_unique() for the qual list from (1). If it
returns true, then outer row matches only the same row from the inner
relation.
3. If uniqueness of outer relation deduces from baserestrictinfo clause like
'x=const' on unique field(s), check that inner has the same clause with the
same constant(s).
4. Compare RowMarks of inner and outer relations. They must have the same
strength.
Some regression tests changed due to self-join removal logic.
[1]
https://www.postgresql.org/message-id/raw/CAApHDvpggnFMC4yP-jUO7PKN%3DfXeErW5bOxisvJ0HvkHQEY%3DWw%40mail.gmail.com
---
doc/src/sgml/config.sgml | 16 +
src/backend/optimizer/path/indxpath.c | 39 +
src/backend/optimizer/plan/analyzejoins.c | 1045 -
src/backend/optimizer/plan/planmain.c |5 +
src/backend/utils/misc/guc.c | 10 +
src/include/optimizer/paths.h |3 +
src/include/optimizer/planmain.h |6 +
src/test/regress/expected/equivclass.out | 32 +
src/test/regress/expected/join.out| 735 +++
src/test/regress/expected/sysviews.out|3 +-
src/test/regress/sql/equivclass.sql | 16 +
src/test/regress/sql/join.sql | 329 +++
src/tools/pgindent/typedefs.list |2 +
13 files changed, 2215 insertions(+), 26 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index a5cd4e44c7..2cfb62f97f 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -5297,6 +5297,22 @@ ANY num_sync (
+ enable_self_join_removal (boolean)
+
+ enable_self_join_removal configuration
parameter
+
+
+
+
+Enables or disables the query planner's optimization which analyses
+query tree and replaces self joins with semantically equivalent single
+scans. Take into consideration only plain tables.
+The default is on.
+
+
+
+
enable_seqscan (boolean)
diff --git a/src/backend/optimizer/path/indxpath.c
b/src/backend/optimizer/path/indxpath.c
index 045ff2e487..a9f8f89312 100644
--- a/src/backend/optimizer/path/indxpath.c
+++ b/src/backend/optimizer/path/indxpath.c
@@ -3495,8 +3495,24 @@ bool
relation_has_unique_index_for(PlannerInfo *root, RelOptInfo *rel,
List *restrictlist,
List *exprlist, List
*oprlist)
+{
+ return relation_has_unique_index_ext(root, rel, restrictlist,
+
exprlist, oprlist, NULL);
+}
+
+/*
+ * relation_has_unique_index_ext
+ * if extra_clauses isn't NULL, return baserestrictinfo clauses which were
+ * used to derive uniqueness.
+ */
+bool
+relation_has_unique_index_ext(PlannerInfo *root, RelOptInfo *rel,
+ List *restrictlist,
+ List *exprlist, List
*oprlist,
+ List **extra_clauses)
{
ListCell *ic;
+ List *exprs;
Assert(list_length(exprlist) == list_length(oprlist));
@@ -3551,6 +3567,8 @@ relation_has_unique_index_for(PlannerInfo *root,
RelOptInfo *rel,
IndexOptInfo *ind = (IndexOptInfo *) lfirst(ic);
int c;
+ exprs = NIL;
+
/*
* If the index is not unique, or not immediately enforced, or
if it's
* a partial index that doesn't match the query, it's useless
here.
@@ -3598,6 +3616,23 @@ relation_has_unique_index_for(PlannerInfo *root,
RelOptInfo *rel,
if (match_index_to_operand(rexpr,
Re: standby promotion can create unreadable WAL
On Tue, Aug 23, 2022 at 12:06 AM Robert Haas wrote:
>
However, if anything
> did try to look at file #4 it would get confused. Maybe that can
> happen if this is a streaming standby, where we only write an
> end-of-recovery record upon promotion, rather than a checkpoint, or
> maybe if there are cascading standbys someone could try to actually
> use the 00020004 file for something. I'm not sure. But
> unless I'm missing something, that file is bogus, and our only hope of
> not having problems is that perhaps no one will ever look at it.
I tried in streaming mode, but it seems in the streaming mode we will
never create this bogus file because of this check [1]. So if the
StandbyMode is true then we are never setting "abortedRecPtr" and
"missingContrecPtr" which means we will never create that 0-filled gap
in the WAL file that we are discussing in this thread.
Do we need to set it? I feel we don't. Why? because on this thread
we are also discussing that if the timeline switches then we don’t
need to create that 0-filled gap and that is the actual problem we are
discussing here. And we know that if we are coming out of the
StandbyMode then we will always switch the timeline so we don’t create
that 0-filled gap. OTOH if we are coming out of the archive recovery
then also we will switch the timeline so in that case also we do not
need that. So practically we need to 0 fill that partial record only
when we are doing the crash recovery is that understanding correct?
If so then we can simply avoid setting these variables if
ArchiveRecoveryRequested is true. So in the below check[1] instead of
(!StandbyMode), we can just put (! ArchiveRecoveryRequested), and then
we don't need any other fix. Am I missing anything?
[1]
ReadRecord{
..record = XLogPrefetcherReadRecord(xlogprefetcher, &errormsg);
if (record == NULL)
{
/*
* When not in standby mode we find that WAL ends in an incomplete
* record, keep track of that record. After recovery is done,
* we’ll write a record to indicate to downstream WAL readers that
* that portion is to be ignored.
*/
if (!StandbyMode &&
!XLogRecPtrIsInvalid(xlogreader->abortedRecPtr))
{
abortedRecPtr = xlogreader->abortedRecPtr;
missingContrecPtr = xlogreader->missingContrecPtr;
}
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
Re: Tracking last scan time
Hi On Thu, 25 Aug 2022 at 01:44, David Rowley wrote: > On Thu, 25 Aug 2022 at 03:03, Bruce Momjian wrote: > > > > On Wed, Aug 24, 2022 at 04:01:21PM +0100, Dave Page wrote: > > > On Wed, 24 Aug 2022 at 15:18, Bruce Momjian wrote: > > > Would it be simpler to allow the sequential and index scan columns > to be > > > cleared so you can look later to see if it is non-zero? Should we > allow > > > > > > I don't think so, because then stat values wouldn't necessarily > correlate with > > > each other, and you wouldn't know when any of them were last reset > unless we > > > started tracking each individual reset. At least now you can see when > they were > > > all reset, and you know they were reset at the same time. > > > > Yeah, true. I was more asking if these two columns are in some way > > special or if people would want a more general solution, and if so, is > > that something we want in core Postgres. > > Back when I used to do a bit of PostgreSQL DBA stuff, I had a nightly > job setup to record the state of pg_stat_all_tables and put that into > another table along with the current date. I then had a view that did > some calculations with col - LAG(col) OVER (PARTITION BY relid ORDER > BY date) to fetch the numerical values for each date. I didn't ever > want to reset the stats because it messes with autovacuum. If you zero > out n_ins_since_vacuum more often than auto-vacuum would trigger, then > bad things happen over time (we should really warn about that in the > docs). > > I don't have a particular opinion about the patch, I'm just pointing > out that there are other ways. Even just writing down the numbers on a > post-it note and coming back in a month to see if they've changed is > enough to tell if the table or index has been used. > There are usually other ways to perform monitoring tasks, but there is something to be said for the convenience of having functionality built in and not having to rely on tools, scripts, or post-it notes :-) > > We do also need to consider now that stats are stored in shared memory > that any fields we add are in RAM. > That is a fair point. I believe this is both minimal, and useful though. I've attached a v2 patch that incorporates Greg's suggestions. -- Dave Page Blog: https://pgsnake.blogspot.com Twitter: @pgsnake EDB: https://www.enterprisedb.com last_scan_v2.diff Description: Binary data
Re: windows cfbot failing: my_perl
Hi, On 2022-08-26 06:55:46 -0500, Justin Pryzby wrote: > The last 20 some consecutive builds failed: > https://cirrus-ci.com/github/postgresql-cfbot/postgresql > > like this: > [09:29:27.711] C:\Program Files (x86)\Windows > Kits\10\Include\10.0.20348.0\ucrt\malloc.h(159,17): error C2065: 'my_perl': > undeclared identifier (compiling source file src/pl/plperl/SPI.c) > [c:\cirrus\plperl.vcxproj] > [09:29:27.711] C:\Program Files (x86)\Windows > Kits\10\Include\10.0.20348.0\ucrt\malloc.h(159,17): error C2065: 'my_perl': > undeclared identifier (compiling source file src/pl/plperl/Util.c) > [c:\cirrus\plperl.vcxproj] > [09:29:27.711] C:\Program Files (x86)\Windows > Kits\10\Include\10.0.20348.0\ucrt\malloc.h(159,17): error C2223: left of > '->IMem' must point to struct/union (compiling source file > src/pl/plperl/SPI.c) [c:\cirrus\plperl.vcxproj] > [09:29:27.711] C:\Program Files (x86)\Windows > Kits\10\Include\10.0.20348.0\ucrt\malloc.h(159,17): error C2223: left of > '->IMem' must point to struct/union (compiling source file > src/pl/plperl/Util.c) [c:\cirrus\plperl.vcxproj] > > I imagine it may be due to an error hit while rebuilding the ci's docker > image. I don't think it's CI specific, see https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=hamerkop&dt=2022-08-26%2011%3A00%3A11 Looks like the failures might have started with https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=121d2d3d70ecdb2113b340c5f3b99a61341291af based on https://cirrus-ci.com/github/postgres/postgres/ Not immediately obvious why that would be. Greetings, Andres Freund
Fix possible bogus array out of bonds (src/backend/access/brin/brin_minmax_multi.c)
Hi, At function has_matching_range, if variable ranges->nranges == 0, we exit quickly with a result equal to false. This means that nranges can be zero. It occurs then that it is possible then to occur an array out of bonds, in the initialization of the variable maxvalue. So if nranges is equal to zero, there is no need to initialize minvalue and maxvalue. The patch tries to fix it, avoiding possible errors by using maxvalue. regards, Ranier Vilela 0001-fix-bogus-possible-array-out-bonds.patch Description: Binary data
Re: standby promotion can create unreadable WAL
On Fri, Aug 26, 2022 at 8:44 AM Dilip Kumar wrote:
> ArchiveRecoveryRequested is true. So in the below check[1] instead of
> (!StandbyMode), we can just put (! ArchiveRecoveryRequested), and then
> we don't need any other fix. Am I missing anything?
>
> [1]
> ReadRecord{
> ..record = XLogPrefetcherReadRecord(xlogprefetcher, &errormsg);
> if (record == NULL)
> {
> /*
> * When not in standby mode we find that WAL ends in an incomplete
> * record, keep track of that record. After recovery is done,
> * we’ll write a record to indicate to downstream WAL readers that
> * that portion is to be ignored.
> */
> if (!StandbyMode &&
> !XLogRecPtrIsInvalid(xlogreader->abortedRecPtr))
> {
> abortedRecPtr = xlogreader->abortedRecPtr;
> missingContrecPtr = xlogreader->missingContrecPtr;
> }
I agree. Testing StandbyMode here seems bogus. I thought initially
that the test should perhaps be for InArchiveRecovery rather than
ArchiveRecoveryRequested, but I see that the code which switches to a
new timeline cares about ArchiveRecoveryRequested, so I think that is
the correct thing to test here as well.
Concretely, I propose the following patch.
--
Robert Haas
EDB: http://www.enterprisedb.com
fix-contrecord-condition-v1.patch
Description: Binary data
Re: windows cfbot failing: my_perl
Hi,
On 2022-08-26 06:21:51 -0700, Andres Freund wrote:
> On 2022-08-26 06:55:46 -0500, Justin Pryzby wrote:
> > The last 20 some consecutive builds failed:
> > https://cirrus-ci.com/github/postgresql-cfbot/postgresql
> >
> > like this:
> > [09:29:27.711] C:\Program Files (x86)\Windows
> > Kits\10\Include\10.0.20348.0\ucrt\malloc.h(159,17): error C2065: 'my_perl':
> > undeclared identifier (compiling source file src/pl/plperl/SPI.c)
> > [c:\cirrus\plperl.vcxproj]
> > [09:29:27.711] C:\Program Files (x86)\Windows
> > Kits\10\Include\10.0.20348.0\ucrt\malloc.h(159,17): error C2065: 'my_perl':
> > undeclared identifier (compiling source file src/pl/plperl/Util.c)
> > [c:\cirrus\plperl.vcxproj]
> > [09:29:27.711] C:\Program Files (x86)\Windows
> > Kits\10\Include\10.0.20348.0\ucrt\malloc.h(159,17): error C2223: left of
> > '->IMem' must point to struct/union (compiling source file
> > src/pl/plperl/SPI.c) [c:\cirrus\plperl.vcxproj]
> > [09:29:27.711] C:\Program Files (x86)\Windows
> > Kits\10\Include\10.0.20348.0\ucrt\malloc.h(159,17): error C2223: left of
> > '->IMem' must point to struct/union (compiling source file
> > src/pl/plperl/Util.c) [c:\cirrus\plperl.vcxproj]
> >
> > I imagine it may be due to an error hit while rebuilding the ci's docker
> > image.
>
> I don't think it's CI specific, see
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=hamerkop&dt=2022-08-26%2011%3A00%3A11
>
> Looks like the failures might have started with
> https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=121d2d3d70ecdb2113b340c5f3b99a61341291af
> based on
> https://cirrus-ci.com/github/postgres/postgres/
>
> Not immediately obvious why that would be.
Reproduces in a VM, it starts to fail with that commit. Looks like somehow
different macros are trampling on each other. Something in perl is interfering
with msvc's malloc.h, turning
if (_Marker == _ALLOCA_S_HEAP_MARKER)
{
free(_Memory);
}
into
if (_Marker == 0x)
{
(*(my_perl->IMem)->pFree)((my_perl->IMem), (_Memory));
}
after preprocessing. No idea how.
Greetings,
Andres Freund
Re: Strip -mmacosx-version-min options from plperl build
On 2022-08-25 Th 18:13, Andres Freund wrote:
> Hi,
>
> On 2022-08-25 18:04:34 -0400, Andrew Dunstan wrote:
>> On 2022-08-25 Th 17:47, Andres Freund wrote:
$ egrep '_PG_init|Pg_magic_func' plperl.i
extern __attribute__((visibility("default"))) void _PG_init(void);
extern __attribute__((visibility("default"))) const Pg_magic_struct
*Pg_magic_func(void); const Pg_magic_struct * Pg_magic_func(void) {
static const Pg_magic_struct Pg_magic_data = { sizeof(Pg_magic_struct),
16 / 100, 100, 32, 64,
_PG_init(void)
>>> Could you show objdump -t of the library? Perhaps once with the flags as
>>> now,
>>> and once relinking with the "old" flags that we're now omitting?
>>
>> current:
>>
>>
>> $ objdump -t plperl.dll | egrep '_PG_init|Pg_magic_func'
>> [103](sec 1)(fl 0x00)(ty 20)(scl 2) (nx 0) 0x40a0
>> Pg_magic_func
>> [105](sec 1)(fl 0x00)(ty 20)(scl 2) (nx 0) 0x40b0 _PG_init
>>
>>
>> from July 11th build:
>>
>>
>> $ objdump -t plperl.dll | egrep '_PG_init|Pg_magic_func'
>> [101](sec 1)(fl 0x00)(ty 20)(scl 2) (nx 0) 0x40d0
>> Pg_magic_func
>> [103](sec 1)(fl 0x00)(ty 20)(scl 2) (nx 0) 0x40e0 _PG_init
> Thanks.
>
> So it looks like it's not the symbol not being exported. I wonder if the image
> base thing is somehow the problem? Sounds like it should just be an efficiency
> difference, by avoiding some relocations, not a functional difference.
>
> Can you try adding just that to the flags for building and whether that then
> allows a LOAD 'plperl' to succeed?
>
Adding what?
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
Re: standby promotion can create unreadable WAL
On 2022-Aug-26, Robert Haas wrote: > I agree. Testing StandbyMode here seems bogus. I thought initially > that the test should perhaps be for InArchiveRecovery rather than > ArchiveRecoveryRequested, but I see that the code which switches to a > new timeline cares about ArchiveRecoveryRequested, so I think that is > the correct thing to test here as well. Yeah, I think you had already established elsewhere that testing StandbyMode was the wrong thing to do. Testing ArchiveRecoveryRequested here seems quite odd at first, but given the copying behavior, I agree that it seems a correct thing to do. There's a small typo in the comment: "When find that". I suppose that was meant to be "When we find that". You end that para with "and thus we should not do this", but that sounds like it wouldn't matter if we did. Maybe "and thus doing this would be wrong, so skip it." or something like that. (Perhaps be even more specific and say "if we did this, we would later create an overwrite record in the wrong place, breaking everything") -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
Re: Insertion Sort Improvements
> convenient if you know the type at compile time. See the attached, > which I had laying around when I was looking at PDQuicksort. I believe > it's similar to what you have in mind. That looks very promising. I also love your recent proposal of partitioning into null and non-null. I suspect that to be a clear winner. > I think it belongs around 10 now anyway. Yeah, I think that change is overdue given modern hardware characteristics (even with the current implementation). Another idea could be to run a binary insertion sort and use a much higher threshold. This could significantly cut down on comparisons (especially with presorted runs, which are quite common in real workloads). If full binary search turned out to be an issue regarding cache locality, we could do it in smaller chunks, e.g. do a micro binary search between the current position (I) and position minus chunk size (say 6-12 or so, whatever fits 1 or 2 cachelines) whenever A[I] < A[I-1] and if we don't find the position within that chunk we continue with the previous chunk, thereby preserving cache locality. With less comparisons we should start keeping track of swaps and use that as an efficient way to determine presortedness. We could change the insertion sort threshold to a swap threshold and do insertion sort until we hit the swap threshold. I suspect that would make the current presorted check obsolete (as binary insertion sort without or even with a few swaps should be faster than the current presorted-check). Cheers, Ben
Re: has_privs_of_role vs. is_member_of_role, redux
On Thu, Aug 25, 2022 at 4:41 PM Tom Lane wrote: > Yeah, I'd lean against back-patching. This is the sort of behavioral > change that users tend not to like finding in minor releases. Here's a small patch. Despite the small size of the patch, there are a couple of debatable points here: 1. Should we have a code comment? I feel it isn't necessary, because there's a comment just a few lines earlier saying "Look up the role OIDs and do permissions checks" and that seems like sufficient justification for what follows. 2. What about the error message? Personally, I'm not very excited about "permission denied to whatever" as a way to phrase an error message. It doesn't sound like particularly good grammar to me. But it's the phrasing we use elsewhere, so I guess we should do the same here. 3. Should we have a test case? We are extremely thin on test cases for NOINHERIT behavior, it seems, and testing this one thing when we don't test anything else seems relatively useless. Also, privileges.sql is a giant mess. It's a 1700+ line test file that tests many fairly unrelated things. I am inclined to think that this file badly needs to be split up into a bunch of smaller files, because it's practically unmaintainable as is. For instance, the stuff at the top of the file is testing a bunch of things about role privileges, but then check some stuff about leakproof functions before coming back to test stuff about groups, which logically probably belongs with the role privileges stuff. Perhaps a reasonable starting split would be something like: - Privileges on roles. - Privileges on relations. - Privileges on other kinds of objects. - Default privileges. - Security barriers and leakproof functions. - Security-restricted operations. Some of those files might be fairly small initially, but they might be get bigger later, especially because it'd be a heck of a lot easier to add new test cases if you didn't have to worry that some change you make is going to break a test 1000 lines down in the file. -- Robert Haas EDB: http://www.enterprisedb.com alterdefaultprivs-v1.patch Description: Binary data
Re: Strip -mmacosx-version-min options from plperl build
Hi, On 2022-08-26 10:04:35 -0400, Andrew Dunstan wrote: > On 2022-08-25 Th 18:13, Andres Freund wrote: > >>> Could you show objdump -t of the library? Perhaps once with the flags as > >>> now, > >>> and once relinking with the "old" flags that we're now omitting? > >> > >> current: > >> > >> > >> $ objdump -t plperl.dll | egrep '_PG_init|Pg_magic_func' > >> [103](sec 1)(fl 0x00)(ty 20)(scl 2) (nx 0) 0x40a0 > >> Pg_magic_func > >> [105](sec 1)(fl 0x00)(ty 20)(scl 2) (nx 0) 0x40b0 _PG_init > >> > >> > >> from July 11th build: > >> > >> > >> $ objdump -t plperl.dll | egrep '_PG_init|Pg_magic_func' > >> [101](sec 1)(fl 0x00)(ty 20)(scl 2) (nx 0) 0x40d0 > >> Pg_magic_func > >> [103](sec 1)(fl 0x00)(ty 20)(scl 2) (nx 0) 0x40e0 _PG_init > > Thanks. > > > > So it looks like it's not the symbol not being exported. I wonder if the > > image > > base thing is somehow the problem? Sounds like it should just be an > > efficiency > > difference, by avoiding some relocations, not a functional difference. > > > > Can you try adding just that to the flags for building and whether that then > > allows a LOAD 'plperl' to succeed? > > > > > Adding what? -Wl,--enable-auto-image-base Greetings, Andres Freund
Re: standby promotion can create unreadable WAL
On Fri, Aug 26, 2022 at 10:06 AM Alvaro Herrera wrote: > There's a small typo in the comment: "When find that". I suppose that > was meant to be "When we find that". You end that para with "and thus > we should not do this", but that sounds like it wouldn't matter if we > did. Maybe "and thus doing this would be wrong, so skip it." or > something like that. (Perhaps be even more specific and say "if we did > this, we would later create an overwrite record in the wrong place, > breaking everything") I think that saying that someone should not do something implies pretty clearly that it would be bad if they did. But I have no problem with your more specific language, and as a general rule, it's good to be specific, so let's use that. v2 attached. Thanks for chiming in. -- Robert Haas EDB: http://www.enterprisedb.com fix-contrecord-condition-v2.patch Description: Binary data
Re: windows cfbot failing: my_perl
Hi,
On 2022-08-26 06:40:47 -0700, Andres Freund wrote:
> On 2022-08-26 06:21:51 -0700, Andres Freund wrote:
> > On 2022-08-26 06:55:46 -0500, Justin Pryzby wrote:
> > > The last 20 some consecutive builds failed:
> > > https://cirrus-ci.com/github/postgresql-cfbot/postgresql
> > >
> > > like this:
> > > [09:29:27.711] C:\Program Files (x86)\Windows
> > > Kits\10\Include\10.0.20348.0\ucrt\malloc.h(159,17): error C2065:
> > > 'my_perl': undeclared identifier (compiling source file
> > > src/pl/plperl/SPI.c) [c:\cirrus\plperl.vcxproj]
> > > [09:29:27.711] C:\Program Files (x86)\Windows
> > > Kits\10\Include\10.0.20348.0\ucrt\malloc.h(159,17): error C2065:
> > > 'my_perl': undeclared identifier (compiling source file
> > > src/pl/plperl/Util.c) [c:\cirrus\plperl.vcxproj]
> > > [09:29:27.711] C:\Program Files (x86)\Windows
> > > Kits\10\Include\10.0.20348.0\ucrt\malloc.h(159,17): error C2223: left of
> > > '->IMem' must point to struct/union (compiling source file
> > > src/pl/plperl/SPI.c) [c:\cirrus\plperl.vcxproj]
> > > [09:29:27.711] C:\Program Files (x86)\Windows
> > > Kits\10\Include\10.0.20348.0\ucrt\malloc.h(159,17): error C2223: left of
> > > '->IMem' must point to struct/union (compiling source file
> > > src/pl/plperl/Util.c) [c:\cirrus\plperl.vcxproj]
> > >
> > > I imagine it may be due to an error hit while rebuilding the ci's docker
> > > image.
> >
> > I don't think it's CI specific, see
> > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=hamerkop&dt=2022-08-26%2011%3A00%3A11
> >
> > Looks like the failures might have started with
> > https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=121d2d3d70ecdb2113b340c5f3b99a61341291af
> > based on
> > https://cirrus-ci.com/github/postgres/postgres/
> >
> > Not immediately obvious why that would be.
>
> Reproduces in a VM, it starts to fail with that commit. Looks like somehow
> different macros are trampling on each other. Something in perl is interfering
> with msvc's malloc.h, turning
>
> if (_Marker == _ALLOCA_S_HEAP_MARKER)
> {
> free(_Memory);
> }
> into
>
> if (_Marker == 0x)
> {
> (*(my_perl->IMem)->pFree)((my_perl->IMem), (_Memory));
> }
>
> after preprocessing. No idea how.
Because perl, extremely unhelpfully, #defines free. Which, not surprisingly,
causes issues when including system headers referencing free as well.
I don't really see a good solution to this other than hoisting the
mb/pg_wchar.h include out to before we include all the perl stuff. That does
fix the issue.
Greetings,
Andres Freund
Re: windows cfbot failing: my_perl
On Fri, Aug 26, 2022 at 9:27 PM Andres Freund wrote: > > Because perl, extremely unhelpfully, #defines free. Which, not surprisingly, > causes issues when including system headers referencing free as well. > > I don't really see a good solution to this other than hoisting the > mb/pg_wchar.h include out to before we include all the perl stuff. That does > fix the issue. We could also move is_valid_ascii somewhere else. It's only tangentially related to "wide chars" anyway. -- John Naylor EDB: http://www.enterprisedb.com
Re: windows cfbot failing: my_perl
Hi, On 2022-08-26 21:39:05 +0700, John Naylor wrote: > On Fri, Aug 26, 2022 at 9:27 PM Andres Freund wrote: > > > > Because perl, extremely unhelpfully, #defines free. Which, not surprisingly, > > causes issues when including system headers referencing free as well. > > > > I don't really see a good solution to this other than hoisting the > > mb/pg_wchar.h include out to before we include all the perl stuff. That does > > fix the issue. > > We could also move is_valid_ascii somewhere else. It's only > tangentially related to "wide chars" anyway. Given the crazy defines of stuff like free, it seems like a good idea to have a rule that no headers should be included after plperl.h with PG_NEED_PERL_XSUB_H defined. It's not like there's not other chances of of pulling in malloc.h from within pg_wchar.h somehow. It's a bit ugly to have the mb/pg_wchar.h in plperl.h instead of plperl_helpers.h, but ... Greetings, Andres Freund
Re: configure --with-uuid=bsd fails on NetBSD
Hi,
On 8/21/22 04:37, Tom Lane wrote:
Andres Freund writes:
Perhaps we should make them error out instead? It doesn't seem helpful to
just return something wrong...
Yeah, might be appropriate.
Based on these discussions, I attached a patch.
Thanks,
Nazir Bilal Yavuzdiff --git a/contrib/uuid-ossp/expected/uuid_ossp_1.out b/contrib/uuid-ossp/expected/uuid_ossp_1.out
new file mode 100644
index 00..4b3d61388c
--- /dev/null
+++ b/contrib/uuid-ossp/expected/uuid_ossp_1.out
@@ -0,0 +1,111 @@
+CREATE EXTENSION "uuid-ossp";
+SELECT uuid_nil();
+ uuid_nil
+--
+ ----
+(1 row)
+
+SELECT uuid_ns_dns();
+ uuid_ns_dns
+--
+ 6ba7b810-9dad-11d1-80b4-00c04fd430c8
+(1 row)
+
+SELECT uuid_ns_url();
+ uuid_ns_url
+--
+ 6ba7b811-9dad-11d1-80b4-00c04fd430c8
+(1 row)
+
+SELECT uuid_ns_oid();
+ uuid_ns_oid
+--
+ 6ba7b812-9dad-11d1-80b4-00c04fd430c8
+(1 row)
+
+SELECT uuid_ns_x500();
+ uuid_ns_x500
+--
+ 6ba7b814-9dad-11d1-80b4-00c04fd430c8
+(1 row)
+
+-- some quick and dirty field extraction functions
+-- this is actually timestamp concatenated with clock sequence, per RFC 4122
+CREATE FUNCTION uuid_timestamp_bits(uuid) RETURNS varbit AS
+$$ SELECT ('x' || substr($1::text, 15, 4) || substr($1::text, 10, 4) ||
+ substr($1::text, 1, 8) || substr($1::text, 20, 4))::bit(80)
+ & x'0FFF3FFF' $$
+LANGUAGE SQL STRICT IMMUTABLE;
+CREATE FUNCTION uuid_version_bits(uuid) RETURNS varbit AS
+$$ SELECT ('x' || substr($1::text, 15, 2))::bit(8) & '' $$
+LANGUAGE SQL STRICT IMMUTABLE;
+CREATE FUNCTION uuid_reserved_bits(uuid) RETURNS varbit AS
+$$ SELECT ('x' || substr($1::text, 20, 2))::bit(8) & '1100' $$
+LANGUAGE SQL STRICT IMMUTABLE;
+CREATE FUNCTION uuid_multicast_bit(uuid) RETURNS bool AS
+$$ SELECT (('x' || substr($1::text, 25, 2))::bit(8) & '0001') != '' $$
+LANGUAGE SQL STRICT IMMUTABLE;
+CREATE FUNCTION uuid_local_admin_bit(uuid) RETURNS bool AS
+$$ SELECT (('x' || substr($1::text, 25, 2))::bit(8) & '0010') != '' $$
+LANGUAGE SQL STRICT IMMUTABLE;
+CREATE FUNCTION uuid_node(uuid) RETURNS text AS
+$$ SELECT substr($1::text, 25) $$
+LANGUAGE SQL STRICT IMMUTABLE;
+-- Ideally, the multicast bit would never be set in V1 output, but the
+-- UUID library may fall back to MC if it can't get the system MAC address.
+-- Also, the local-admin bit might be set (if so, we're probably inside a VM).
+-- So we can't test either bit here.
+SELECT uuid_version_bits(uuid_generate_v1()),
+ uuid_reserved_bits(uuid_generate_v1());
+ERROR: NetBSD's uuid_create function generates version-4 UUIDs instead of version-1
+-- Although RFC 4122 only requires the multicast bit to be set in V1MC style
+-- UUIDs, our implementation always sets the local-admin bit as well.
+SELECT uuid_version_bits(uuid_generate_v1mc()),
+ uuid_reserved_bits(uuid_generate_v1mc()),
+ uuid_multicast_bit(uuid_generate_v1mc()),
+ uuid_local_admin_bit(uuid_generate_v1mc());
+ERROR: NetBSD's uuid_create function generates version-4 UUIDs instead of version-1
+-- timestamp+clock sequence should be monotonic increasing in v1
+SELECT uuid_timestamp_bits(uuid_generate_v1()) < uuid_timestamp_bits(uuid_generate_v1());
+ERROR: NetBSD's uuid_create function generates version-4 UUIDs instead of version-1
+SELECT uuid_timestamp_bits(uuid_generate_v1mc()) < uuid_timestamp_bits(uuid_generate_v1mc());
+ERROR: NetBSD's uuid_create function generates version-4 UUIDs instead of version-1
+-- Ideally, the node value is stable in V1 addresses, but OSSP UUID
+-- falls back to V1MC behavior if it can't get the system MAC address.
+SELECT CASE WHEN uuid_multicast_bit(uuid_generate_v1()) AND
+ uuid_local_admin_bit(uuid_generate_v1()) THEN
+ true -- punt, no test
+ ELSE
+ uuid_node(uuid_generate_v1()) = uuid_node(uuid_generate_v1())
+ END;
+ERROR: NetBSD's uuid_create function generates version-4 UUIDs instead of version-1
+-- In any case, V1MC node addresses should be random.
+SELECT uuid_node(uuid_generate_v1()) <> uuid_node(uuid_generate_v1mc());
+ERROR: NetBSD's uuid_create function generates version-4 UUIDs instead of version-1
+SELECT uuid_node(uuid_generate_v1mc()) <> uuid_node(uuid_generate_v1mc());
+ERROR: NetBSD's uuid_create function generates version-4 UUIDs instead of version-1
+SELECT uuid_generate_v3(uuid_ns_dns(), 'www.widgets.com');
+ uuid_generate_v3
+--
+ 3d813cbb-47fb-32ba-91df-831e1593ac29
+(1 row)
+
+SELECT uuid_generate_v5(uuid_ns_dns(), 'www.widgets.com');
+ uuid_generate_v5
+--
+ 21f7f8
pg_has_role's handling of ADMIN OPTION is pretty weird
According to pg_has_role, it's possible to have USAGE WITH ADMIN
OPTION on a role without having USAGE:
template1=# create role foo;
CREATE ROLE
template1=# create role admin;
CREATE ROLE
template1=# grant foo to admin with inherit false, admin true;
GRANT ROLE
template1=# select p.priv, pg_has_role('admin', 'foo', p.priv) from
(values ('USAGE'), ('MEMBER'),('USAGE WITH ADMIN OPTION'), ('MEMBER
WITH ADMIN OPTION')) p(priv);
priv | pg_has_role
--+-
USAGE| f
MEMBER | t
USAGE WITH ADMIN OPTION | t
MEMBER WITH ADMIN OPTION | t
(4 rows)
To me it seems wrong to say that you can have "X WITH Y" without
having X. If I order a hamburger with fries, I do not only get fries:
I also get a hamburger. I think the problem here is that pg_has_role()
is defined to work like has_table_privilege(), and for table
privileges, each underlying privilege bit has a corresponding bit
representing the right to grant that privilege, and you can't grant
the right to set the privilege without first granting the privilege.
For roles, you just get ADMIN OPTION on the role, and that entitles
you to grant or revoke any privilege associated with the role. So the
whole way this function is defined seems wrong to me. It seems like it
would be more reasonable to have the third argument be, e.g. MEMBER,
USAGE, or ADMIN and forget about this WITH ADMIN OPTION stuff. That
would be a behavior change, though.
If we don't do that, then I think things just get weirder if we add
some more privileges around role memberships. Let's say that in
addition to INHERIT OPTION and GRANT OPTION, we add some other things
that one role could do to another, let's say FLUMMOX, PERTURB, and
DISCOMBOBULATE, then we'll just end up with more and more synonyms for
"does this role have admin option". That is:
column1 | column2
--+-
USAGE| Is this grant inheritable?
MEMBER | Does a grant even exist in the first place?
FLUMMOX | Can this grant flummox?
PERTURB | Can this grant perturb?
DISCOMBOBULATE | Can this grant discombobulate?
USAGE WITH ADMIN OPTION | Does this grant have ADMIN OPTION?
MEMBER WITH ADMIN OPTION | Does this grant have ADMIN OPTION?
FLUMMOX WITH ADMIN OPTION| Does this grant have ADMIN OPTION?
PERTURB WITH ADMIN OPTION| Does this grant have ADMIN OPTION?
DISCOMBOBULATE WITH ADMIN OPTION | Does this grant have ADMIN OPTION?
Maybe everybody else thinks that would be just fine? To me it seems
fairly misleading.
--
Robert Haas
EDB: http://www.enterprisedb.com
Re: standby promotion can create unreadable WAL
>I agree. Testing StandbyMode here seems bogus. I thought initially >that the test should perhaps be for InArchiveRecovery rather than >ArchiveRecoveryRequested, but I see that the code which switches to a >new timeline cares about ArchiveRecoveryRequested, so I think that is >the correct thing to test here as well. >Concretely, I propose the following patch. This patch looks similar to the change suggested in https://www.postgresql.org/message-id/FB0DEA0B-E14E-43A0-811F-C1AE93D00FF3%40amazon.com to deal with panics after promoting a standby. The difference is the patch tests !ArchiveRecoveryRequested instead of !StandbyModeRequested as proposed in the mentioned thread. Thanks -- Sami Imseih Amazon Web Services
Re: making relfilenodes 56 bits
On Fri, Aug 26, 2022 at 7:01 AM Dilip Kumar wrote: > While working on this solution I noticed one issue. Basically, the > problem is that during binary upgrade when we try to rewrite a heap we > would expect that “binary_upgrade_next_heap_pg_class_oid” and > “binary_upgrade_next_heap_pg_class_relfilenumber” are already set for > creating a new heap. But we are not preserving anything so we don't > have those values. One option to this problem is that we can first > start the postmaster in non-binary upgrade mode perform all conflict > checking and rewrite and stop the postmaster. Then start postmaster > again and perform the restore as we are doing now. Although we will > have to start/stop the postmaster one extra time we have a solution. Yeah, that seems OK. Or we could add a new function, like binary_upgrade_allow_relation_oid_and_relfilenode_assignment(bool). Not sure which way is better. > But while thinking about this I started to think that since now we are > completely decoupling the concept of Oid and relfilenumber then > logically during REWRITE we should only be allocating new > relfilenumber but we don’t really need to allocate the Oid at all. > Yeah, we can do that if inside make_new_heap() if we pass the > OIDOldHeap to heap_create_with_catalog(), then it will just create new > storage(relfilenumber) but not a new Oid. But the problem is that the > ATRewriteTable() and finish_heap_swap() functions are completely based > on the relation cache. So now if we only create a new relfilenumber > but not a new Oid then we will have to change this infrastructure to > copy at smgr level. I think it would be a good idea to continue preserving the OIDs. If nothing else, it makes debugging way easier, but also, there might be user-defined regclass columns or something. Note the comments in check_for_reg_data_type_usage(). -- Robert Haas EDB: http://www.enterprisedb.com
Re: Strip -mmacosx-version-min options from plperl build
Andres Freund writes: > On 2022-08-26 10:04:35 -0400, Andrew Dunstan wrote: >> On 2022-08-25 Th 18:13, Andres Freund wrote: >>> Can you try adding just that to the flags for building and whether that then >>> allows a LOAD 'plperl' to succeed? >> Adding what? > -Wl,--enable-auto-image-base And if that doesn't help, try -Wl,--export-all-symbols regards, tom lane
Re: standby promotion can create unreadable WAL
On Fri, Aug 26, 2022 at 11:59 AM Imseih (AWS), Sami wrote: > >I agree. Testing StandbyMode here seems bogus. I thought initially > >that the test should perhaps be for InArchiveRecovery rather than > >ArchiveRecoveryRequested, but I see that the code which switches to a > >new timeline cares about ArchiveRecoveryRequested, so I think that is > >the correct thing to test here as well. > > >Concretely, I propose the following patch. > > This patch looks similar to the change suggested in > https://www.postgresql.org/message-id/FB0DEA0B-E14E-43A0-811F-C1AE93D00FF3%40amazon.com > to deal with panics after promoting a standby. > > The difference is the patch tests !ArchiveRecoveryRequested instead > of !StandbyModeRequested as proposed in the mentioned thread. OK, I didn't realize this bug had been independently discovered and it looks like I was even involved in the previous discussion. I just totally forgot about it. I think, however, that your fix is wrong and this one is right. Fundamentally, the server is either in normal running, or crash recovery, or archive recovery. Standby mode is just an optional behavior of archive recovery, controlling whether or not we keep retrying once the end of WAL is reached. But there's no reason why the server should put the contrecord at a different location when recovery ends depending on that retry behavior. The only thing that matters is whether we're going to switch timelines. -- Robert Haas EDB: http://www.enterprisedb.com
Re: configure --with-uuid=bsd fails on NetBSD
Nazir Bilal Yavuz writes:
> Based on these discussions, I attached a patch.
This is the wrong way to go about it:
+#if defined(__NetBSD__)
+ ereport(ERROR, errmsg("NetBSD's uuid_create function generates "
+ "version-4 UUIDs
instead of version-1"));
+#endif
Older versions of NetBSD generated v1, so you'd incorrectly break
things on those. And who knows whether they might reconsider
in the future?
I think the right fix is to call uuid_create and then actually check
the version field of the result. This avoids breaking what need not
be broken, and it'd also guard against comparable problems on other
platforms (so don't blame NetBSD specifically in the message, either).
regards, tom lane
Re: postgres_fdw hint messages
On Thu, Aug 25, 2022 at 9:42 AM Peter Eisentraut wrote: > The postgres_fdw tests contain this (as amended by patch 0001): > > ALTER SERVER loopback_nopw OPTIONS (ADD password 'dummypw'); > ERROR: invalid option "password" > HINT: Valid options in this context are: service, passfile, > channel_binding, connect_timeout, dbname, host, hostaddr, port, options, > application_name, keepalives, keepalives_idle, keepalives_interval, > keepalives_count, tcp_user_timeout, sslmode, sslcompression, sslcert, > sslkey, sslrootcert, sslcrl, sslcrldir, sslsni, requirepeer, > ssl_min_protocol_version, ssl_max_protocol_version, gssencmode, > krbsrvname, gsslib, target_session_attrs, use_remote_estimate, > fdw_startup_cost, fdw_tuple_cost, extensions, updatable, truncatable, > fetch_size, batch_size, async_capable, parallel_commit, keep_connections > > This annoys developers who are working on libpq connection options, > because any option added, removed, or changed causes this test to need > to be updated. > > It's also questionable how useful this hint is in its current form, > considering how long it is and that the options are in an > implementation-dependent order. I think the place to list the legal options is in the documentation, not the HINT. -- Robert Haas EDB: http://www.enterprisedb.com
Re: Letter case of "admin option"
Here's a patch changing all occurrences of "admin option" in error messages to "ADMIN OPTION". Two of these five messages also exist in previous releases; the other three are new. I'm not sure if this is our final conclusion on what we want to do here, so please speak up if you don't agree. Thanks, -- Robert Haas EDB: http://www.enterprisedb.com capitalize-admin-option-v1.patch Description: Binary data
Re: postgres_fdw hint messages
Robert Haas writes: > On Thu, Aug 25, 2022 at 9:42 AM Peter Eisentraut > wrote: >> HINT: Valid options in this context are: service, passfile, >> channel_binding, connect_timeout, dbname, host, hostaddr, port, options, >> application_name, keepalives, keepalives_idle, keepalives_interval, >> keepalives_count, tcp_user_timeout, sslmode, sslcompression, sslcert, >> sslkey, sslrootcert, sslcrl, sslcrldir, sslsni, requirepeer, >> ssl_min_protocol_version, ssl_max_protocol_version, gssencmode, >> krbsrvname, gsslib, target_session_attrs, use_remote_estimate, >> fdw_startup_cost, fdw_tuple_cost, extensions, updatable, truncatable, >> fetch_size, batch_size, async_capable, parallel_commit, keep_connections >> >> This annoys developers who are working on libpq connection options, >> because any option added, removed, or changed causes this test to need >> to be updated. >> >> It's also questionable how useful this hint is in its current form, >> considering how long it is and that the options are in an >> implementation-dependent order. > I think the place to list the legal options is in the documentation, > not the HINT. I think listing them in a hint is reasonable as long as the hint doesn't get longer than a line or two. This one is entirely out of hand, so I agree with just dropping it. Note that there is essentially identical code in dblink, file_fdw, and backend/foreign/foreign.c. Do we want to nuke them all? Or maybe make a policy decision to suppress such HINTs when there are more than ~10 matches? (The latter policy would likely eventually end by always suppressing everything...) Peter also mentioned the possibility of "did you mean" with a closest match offered. That seems like a reasonable idea if someone is motivated to create the code, which I'm not. I vote for just dropping all these hints for now, while leaving the door open for anyone who wants to write closest-match-offering code. regards, tom lane
Re: SQL/JSON features for v15
On 8/24/22 8:16 PM, Andrew Dunstan wrote: On 2022-08-24 We 20:05, Nikita Glukhov wrote: v8 - is a highly WIP patch, which I failed to finish today. Even some test cases fail now, and they simply show unfinished things like casts to bytea (they can be simply removed) and missing safe input functions. Thanks for your work, please keep going. Thanks for the efforts Nikita. With RMT hat on, I want to point out that it's nearing the end of the week, and if we are going to go forward with this path, we do need to review soon. The Beta 4 release date is set to 9/8, and if we are going to commit or revert, we should leave enough time to ensure that we have enough time to review and the patches are able to successfully get through the buildfarm. Thanks, Jonathan OpenPGP_signature Description: OpenPGP digital signature
Re: Use array as object (src/fe_utils/parallel_slot.c)
Em dom., 21 de ago. de 2022 às 21:15, Michael Paquier escreveu: > On Fri, Aug 19, 2022 at 02:22:32PM -0500, Justin Pryzby wrote: > > If you trace the history back to a17923204, you'll see a comment about > the > > "zeroth slot", which makes it clear that the first slot it what's > intended. > > > > I agree that it would be clearer if this were written as > slots[0].connection. > > Based on the way the code is written on HEAD, this would be the > correct assumption. Now, calling PQgetCancel() would return NULL for > a connection that we actually ignore in the code a couple of lines > above when it has PGINVALID_SOCKET. So it seems to me that the > suggestion of using "cancelconn", which would be the first valid > connection, rather than always the first connection, which may be > using an invalid socket, is correct, so as we always have our hands > on a way to cancel a command. > Thanks Michael, for looking at this. Is it worth creating a commiffest? regards, Ranier Vilela
Re: pg_has_role's handling of ADMIN OPTION is pretty weird
On Fri, Aug 26, 2022 at 11:55 AM Robert Haas wrote: > According to pg_has_role, it's possible to have USAGE WITH ADMIN > OPTION on a role without having USAGE: One more thing about this. The documentation about how this function actually works seems never to have been very good, and I think it's actually worse starting in v13. In v12 and prior it wasn't terribly clear, but we said this: "pg_has_role checks whether a user can access a role in a particular way. Its argument possibilities are analogous to has_table_privilege, except that public is not allowed as a user name. The desired access privilege type must evaluate to some combination of MEMBER or USAGE. MEMBER denotes direct or indirect membership in the role (that is, the right to do SET ROLE), while USAGE denotes whether the privileges of the role are immediately available without doing SET ROLE." Now, has_table_privilege() allows you to specify multiple table options and to append WITH GRANT OPTION to any or all of them. That actually works for pg_has_role() too, and a particularly sharp user might suppose based on what we say elsewhere in the documentation that, in the case of roles, we normally write WITH ADMIN OPTION rather than WITH GRANT OPTION. So possibly someone could figure out what this function actually does without reading the source code, at least if they have a PhD degree in PostgreSQL-ology. Starting in v13, the only explicit mention of pg_has_role() is this table entry: "pg_has_role ( [ user name or oid, ] role text or oid, privilege text ) → boolean Does user have privilege for role? Allowable privilege types are MEMBER and USAGE. MEMBER denotes direct or indirect membership in the role (that is, the right to do SET ROLE), while USAGE denotes whether the privileges of the role are immediately available without doing SET ROLE. This function does not allow the special case of setting user to public, because the PUBLIC pseudo-role can never be a member of real roles." That gives no hint that you can specify multiple privileges, let alone append WITH ADMIN OPTION or WITH GRANT OPTION. Everything else in this table has the same problem. There is some text above the table which explains what's going on here and from which it might be possible to infer the behavior of pg_has_role(), but only if you actually read that text and understand that it actually acts as a modifier to everything as follows. None of the functions actually do what they say they do; they all do approximately that, but as modified to fit the scheme described in this paragraph. At the very least, these table entries should say that the last argument is called "privileges" not "privilege" so that someone might have a clue that more than one can be specified. And for the ones where you can add "WITH GRANT OPTION" or "WITH ADMIN OPTION" that should be mentioned in the table itself. -- Robert Haas EDB: http://www.enterprisedb.com
Re: use ARM intrinsics in pg_lfind32() where available
On Thu, Aug 25, 2022 at 11:13:47PM -0700, Nathan Bossart wrote:
> Here is a new patch set that applies on top of v9-0001 in the
> json_lex_string patch set [0] and v3 of the is_valid_ascii patch [1].
Here is a rebased patch set that applies to HEAD.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
>From 8d8afe70bccec20cd381934fae5e11e155d78129 Mon Sep 17 00:00:00 2001
From: Nathan Bossart
Date: Thu, 25 Aug 2022 22:18:30 -0700
Subject: [PATCH v4 1/2] abstract architecture-specific implementation details
from pg_lfind32()
---
src/include/port/pg_lfind.h | 55 +---
src/include/port/simd.h | 63 +
2 files changed, 93 insertions(+), 25 deletions(-)
diff --git a/src/include/port/pg_lfind.h b/src/include/port/pg_lfind.h
index a4e13dffec..7a851ea42c 100644
--- a/src/include/port/pg_lfind.h
+++ b/src/include/port/pg_lfind.h
@@ -91,16 +91,19 @@ pg_lfind32(uint32 key, uint32 *base, uint32 nelem)
{
uint32 i = 0;
-#ifdef USE_SSE2
+#ifndef USE_NO_SIMD
/*
- * A 16-byte register only has four 4-byte lanes. For better
- * instruction-level parallelism, each loop iteration operates on a block
- * of four registers. Testing has showed this is ~40% faster than using a
- * block of two registers.
+ * For better instruction-level parallelism, each loop iteration operates
+ * on a block of four registers. Testing for SSE2 has showed this is ~40%
+ * faster than using a block of two registers.
*/
- const __m128i keys = _mm_set1_epi32(key); /* load 4 copies of key */
- uint32 iterations = nelem & ~0xF; /* round down to multiple of 16 */
+ const Vector32 keys = vector32_broadcast(key); /* load copies of key */
+ uint32 nelem_per_vector = sizeof(Vector32) / sizeof(uint32);
+ uint32 nelem_per_iteration = 4 * nelem_per_vector;
+
+ /* round down to multiple of elements per iteration */
+ uint32 tail_idx = nelem & ~(nelem_per_iteration - 1);
#if defined(USE_ASSERT_CHECKING)
bool assert_result = false;
@@ -116,31 +119,33 @@ pg_lfind32(uint32 key, uint32 *base, uint32 nelem)
}
#endif
- for (i = 0; i < iterations; i += 16)
+ for (i = 0; i < tail_idx; i += nelem_per_iteration)
{
- /* load the next block into 4 registers holding 4 values each */
- const __m128i vals1 = _mm_loadu_si128((__m128i *) & base[i]);
- const __m128i vals2 = _mm_loadu_si128((__m128i *) & base[i + 4]);
- const __m128i vals3 = _mm_loadu_si128((__m128i *) & base[i + 8]);
- const __m128i vals4 = _mm_loadu_si128((__m128i *) & base[i + 12]);
+ Vector32 vals1, vals2, vals3, vals4,
+ result1, result2, result3, result4,
+ tmp1, tmp2, result;
+
+ /* load the next block into 4 registers */
+ vector32_load(&vals1, &base[i]);
+ vector32_load(&vals2, &base[i + nelem_per_vector]);
+ vector32_load(&vals3, &base[i + nelem_per_vector * 2]);
+ vector32_load(&vals4, &base[i + nelem_per_vector * 3]);
/* compare each value to the key */
- const __m128i result1 = _mm_cmpeq_epi32(keys, vals1);
- const __m128i result2 = _mm_cmpeq_epi32(keys, vals2);
- const __m128i result3 = _mm_cmpeq_epi32(keys, vals3);
- const __m128i result4 = _mm_cmpeq_epi32(keys, vals4);
+ result1 = vector32_eq(keys, vals1);
+ result2 = vector32_eq(keys, vals2);
+ result3 = vector32_eq(keys, vals3);
+ result4 = vector32_eq(keys, vals4);
/* combine the results into a single variable */
- const __m128i tmp1 = _mm_or_si128(result1, result2);
- const __m128i tmp2 = _mm_or_si128(result3, result4);
- const __m128i result = _mm_or_si128(tmp1, tmp2);
+ tmp1 = vector32_or(result1, result2);
+ tmp2 = vector32_or(result3, result4);
+ result = vector32_or(tmp1, tmp2);
/* see if there was a match */
- if (_mm_movemask_epi8(result) != 0)
+ if (vector32_any_lane_set(result))
{
-#if defined(USE_ASSERT_CHECKING)
Assert(assert_result == true);
-#endif
return true;
}
}
@@ -151,14 +156,14 @@ pg_lfind32(uint32 key, uint32 *base, uint32 nelem)
{
if (key == base[i])
{
-#if defined(USE_SSE2) && defined(USE_ASSERT_CHECKING)
+#ifndef USE_NO_SIMD
Assert(assert_result == true);
#endif
return true;
}
}
-#if defined(USE_SSE2) && defined(USE_ASSERT_CHECKING)
+#ifndef USE_NO_SIMD
Assert(assert_result == false);
#endif
return false;
diff --git a/src/include/port/simd.h b/src/include/port/simd.h
index a425cd887b..c42dccf784 100644
--- a/src/include/port/simd.h
+++ b/src/include/port/simd.h
@@ -31,6 +31,7 @@
#include
#define USE_SSE2
typedef __m128i Vector8;
+typedef __m128i Vector32;
#else
/*
@@ -39,14 +40,17 @@ typedef __m128i Vector8;
*/
#define USE_NO_SIMD
typedef uint64 Vector8;
+typedef uint64 Vector32;
#endif
/* load/store operations */
static inline void vector8_load(Vector8 *v, const uint8 *s);
+static inline void vector32_load(Vector32 *v, const uint32 *s);
/* assignment operations */
static inline Vector8 vector8_broadcast(const uint8 c);
+static inline Vector32 vector32_broadcast(const uint3
Re: replacing role-level NOINHERIT with a grant-level option
On Thu, Aug 25, 2022 at 10:19 AM Robert Haas wrote: > On Wed, Aug 24, 2022 at 10:23 AM tushar wrote: > > On 8/24/22 12:28 AM, Robert Haas wrote: > > > This patch needed to be rebased pretty extensively after commit > > > ce6b672e4455820a0348214be0da1a024c3f619f. Here is a new version. > > Thanks, Robert, I have retested this patch with my previous scenarios > > and things look good to me. > > I read through this again and found a comment that needed to be > updated, so I did that, bumped catversion, and committed this. Upon further review, this patch failed to update the documentation as thoroughly as would have been desirable. Here is a follow-up patch to correct a few oversights. -- Robert Haas EDB: http://www.enterprisedb.com v1-0001-docs-Fix-up-some-out-of-date-references-to-INHERI.patch Description: Binary data
Re: standby promotion can create unreadable WAL
>I think, however, that your fix is wrong and this one is right. >Fundamentally, the server is either in normal running, or crash >recovery, or archive recovery. Standby mode is just an optional >behavior of archive recovery Good point. Thanks for clearing my understanding. Thanks -- Sami Imseih Amazon Web Services
Re: SQL/JSON features for v15
On 2022-08-26 Fr 12:36, Jonathan S. Katz wrote: > On 8/24/22 8:16 PM, Andrew Dunstan wrote: >> >> On 2022-08-24 We 20:05, Nikita Glukhov wrote: >>> >>> >>> v8 - is a highly WIP patch, which I failed to finish today. >>> Even some test cases fail now, and they simply show unfinished >>> things like casts to bytea (they can be simply removed) and missing >>> safe input functions. >>> >> >> Thanks for your work, please keep going. > > Thanks for the efforts Nikita. > > With RMT hat on, I want to point out that it's nearing the end of the > week, and if we are going to go forward with this path, we do need to > review soon. The Beta 4 release date is set to 9/8, and if we are > going to commit or revert, we should leave enough time to ensure that > we have enough time to review and the patches are able to successfully > get through the buildfarm. > > Also I'm going to be traveling and more or less offline from Sept 5th, so if I'm going to be involved we'd need a decision by Sept 1st or 2nd, I think, so time is running very short. Of course, others could do the required commit work either way a bit later, but not much. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
Re: Strip -mmacosx-version-min options from plperl build
On 2022-08-26 Fr 12:11, Tom Lane wrote: > Andres Freund writes: >> On 2022-08-26 10:04:35 -0400, Andrew Dunstan wrote: >>> On 2022-08-25 Th 18:13, Andres Freund wrote: Can you try adding just that to the flags for building and whether that then allows a LOAD 'plperl' to succeed? >>> Adding what? >> -Wl,--enable-auto-image-base didn't work > And if that doesn't help, try -Wl,--export-all-symbols worked cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
Re: Strip -mmacosx-version-min options from plperl build
Andrew Dunstan writes: > On 2022-08-26 Fr 12:11, Tom Lane wrote: >> And if that doesn't help, try -Wl,--export-all-symbols > worked Hmph. Hard to see how that isn't a linker bug. As a stopgap to get the farm green again, I propose adding something like ifeq ($(PORTNAME), cygwin) SHLIB_LINK += -Wl,--export-all-symbols endif to plperl's makefile. regards, tom lane
Re: Add 64-bit XIDs into PostgreSQL 15
On Wed, Jan 05, 2022 at 06:12:26PM -0600, Justin Pryzby wrote: > On Wed, Jan 05, 2022 at 06:51:37PM -0500, Bruce Momjian wrote: > > On Tue, Jan 4, 2022 at 10:22:50PM +, Finnerty, Jim wrote: > > > I'm concerned about the maintainability impact of having 2 new > > > on-disk page formats. It's already complex enough with XIDs and > > > multixact-XIDs. > > > > > > If the lack of space for the two epochs in the special data area is > > > a problem only in an upgrade scenario, why not resolve the problem > > > before completing the upgrade process like a kind of post-process > > > pg_repack operation that converts all "double xmax" pages to > > > the "double-epoch" page format? i.e. maybe the "double xmax" > > > representation is needed as an intermediate representation during > > > upgrade, but after upgrade completes successfully there are no pages > > > with the "double-xmax" representation. This would eliminate a whole > > > class of coding errors and would make the code dealing with 64-bit > > > XIDs simpler and more maintainable. > > > > Well, yes, we could do this, and it would avoid the complexity of having > > to support two XID representations, but we would need to accept that > > fast pg_upgrade would be impossible in such cases, since every page > > would need to be checked and potentially updated. > > > > You might try to do this while the server is first started and running > > queries, but I think we found out from the online checkpoint patch that > > I think you meant the online checksum patch. Which this reminded me of, too. I wondered whether anyone had considered using relation forks to maintain state of these long, transitional processes. Either a whole new fork, or additional bits in the visibility map, which has page-level bits. There'd still need to be a flag somewhere indicating whether checksums/xid64s/etc were enabled cluster-wide. The VM/fork bits would need to be checked while the cluster was being re-processed online. This would add some overhead. After the cluster had reached its target state, the flag could be set, and the VM bits would no longer need to be checked. -- Justin
Re: Strip -mmacosx-version-min options from plperl build
On 2022-08-26 Fr 16:00, Tom Lane wrote: > Andrew Dunstan writes: >> On 2022-08-26 Fr 12:11, Tom Lane wrote: >>> And if that doesn't help, try -Wl,--export-all-symbols >> worked > Hmph. Hard to see how that isn't a linker bug. As a stopgap > to get the farm green again, I propose adding something like > > ifeq ($(PORTNAME), cygwin) > SHLIB_LINK += -Wl,--export-all-symbols > endif > > to plperl's makefile. > > +1 cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
Re: HOT chain validation in verify_heapam()
Hi,
Thanks for working on this.
+htup = (HeapTupleHeader) PageGetItem(ctx.page, rditem);
+if (!(HeapTupleHeaderIsHeapOnly(htup) &&
htup->t_infomask & HEAP_UPDATED))
+report_corruption(&ctx,
+ psprintf("Redirected Tuple at
line pointer offset %u is not HEAP_ONLY_TUPLE or HEAP_UPDATED tuple",
+ (unsigned) rdoffnum));
This isn't safe because the line pointer referenced by rditem may not
have been sanity-checked yet. Refer to the comment just below where it
says "Sanity-check the line pointer's offset and length values".
There are multiple problems with this error message. First, if you
take a look at the existing messages - which is always a good thing to
do when adding new ones - you will see that they are capitalized
differently. Try to match the existing style. Second, we made a real
effort with the existing messages to avoid referring to the names of
identifiers that only exist at the C level. For example, just above
you will see a message which says "line pointer redirection to item at
offset %u precedes minimum offset %u". It deliberately does not say
"line pointer redirection to item at offset %u is less than
FirstOffsetNumber" even though that would be an equally correct
statement of the problem. The intent here is to make the messages at
least somewhat accessible to users who are somewhat familiar with how
PostgreSQL storage works but may not read the C code. These comments
apply to every message you add in the patch.
The message also does not match the code. The code tests for
HEAP_UPDATED, but the message claims that the code is testing for
either HEAP_ONLY_TUPLE or HEAP_UPDATED. As a general rule, it is best
not to club related tests together in cases like this, because it
enables better and more specific error messages.
It would be clearer to make an explicit comparison to 0, like
(htup->t_infomask & HEAP_UPDATED) != 0, rather than relying on 0 being
false and non-zero being true. It doesn't matter to the compiler, but
it may help human readers.
+/*
+ * Add line pointer offset to predecessor array.
+ * 1) If xmax is matching with xmin of next
Tuple(reaching via its t_ctid).
+ * 2) If next tuple is in the same page.
+ * Raise corruption if:
+ * We have two tuples having same predecessor.
+ *
+ * We add offset to predecessor irrespective of
transaction(t_xmin) status. We will
+ * do validation related to transaction status(and also
all other validations)
+ * when we loop over predecessor array.
+ */
The formatting of this comment will, I think, be mangled if pgindent
is run against the file. You can use - markers to prevent that, I
believe, or (better) write this as a paragraph without relying on the
lines ending up uneven in length.
+if (predecessor[nextTupOffnum] != 0)
+{
+report_corruption(&ctx,
+psprintf("Tuple at offset %u is
reachable from two or more updated tuple",
+(unsigned) nextTupOffnum));
+continue;
+}
You need to do this test after xmin/xmax matching. Otherwise you might
get false positives. Also, the message should be more specific and
match the style of the existing messages. ctx.offnum is already going
to be reported in another column, but we can report both nextTupOffnum
and predecessor[nextTupOffnum] here e.g. "updated version at offset %u
is also the updated version of tuple at offset %u".
+currTupXmax = HeapTupleHeaderGetUpdateXid(ctx.tuphdr);
+lp = PageGetItemId(ctx.page, nextTupOffnum);
+
+htup = (HeapTupleHeader) PageGetItem(ctx.page, lp);
This has the same problem I mentioned in my first comment above,
namely, we haven't necessarily sanity-checked the length and offset
values for nextTupOffnum yet. Saying that another way, if the contents
of lp are corrupt and point off the page, we want that to be reported
as corruption (which the current code will already do) and we want
this check to be skipped so that we don't crash or access random
memory addresses. You need to think about how to rearrange the code so
that we only perform checks that are known to be safe.
+/* Now loop over offset and validate data in predecessor array.*/
+for ( ctx.offnum = FirstOffsetNumber; ctx.offnum <= maxoff;
+ ctx.offnum = OffsetNumberNext(ctx.offnum))
Please take the time to format your code according to the PostgeSQL
standard practice. If you don't know what that looks like, use
pgindent.
+{
+HeapTupleHeader pred_htup, curr_htup;
+TransactionId pred_xmin, curr_xmin, curr_xmax;
+ItemId pred_lp, curr_lp;
Same here.
Within this loop, y
Re: Strip -mmacosx-version-min options from plperl build
Hi, On 2022-08-26 16:00:31 -0400, Tom Lane wrote: > Andrew Dunstan writes: > > On 2022-08-26 Fr 12:11, Tom Lane wrote: > >> And if that doesn't help, try -Wl,--export-all-symbols > > > worked Except that it's only happening for plperl, I'd wonder if it's possibly related to our magic symbols being prefixed with _. I noticed that the underscore prefix e.g. changes the behaviour of gcc's "collect2" on AIX, which is responsible for exporting symbols etc. > Hmph. Hard to see how that isn't a linker bug. Agreed, given that this is only happening with plperl, and not with any of the other extensions... > As a stopgap to get the farm green again, I propose adding something like > > ifeq ($(PORTNAME), cygwin) > SHLIB_LINK += -Wl,--export-all-symbols > endif > > to plperl's makefile. :( Greetings, Andres Freund
Re: windows cfbot failing: my_perl
Hi, Tom, Ilmari, you seem to have hacked on this stuff most (not so) recently. Do you have a better suggestion than moving the mb/pg_wchar.h include out of plperl_helpers.h as I suggest below? On 2022-08-26 07:47:40 -0700, Andres Freund wrote: > On 2022-08-26 21:39:05 +0700, John Naylor wrote: > > On Fri, Aug 26, 2022 at 9:27 PM Andres Freund wrote: > > > > > > Because perl, extremely unhelpfully, #defines free. Which, not > > > surprisingly, > > > causes issues when including system headers referencing free as well. > > > > > > I don't really see a good solution to this other than hoisting the > > > mb/pg_wchar.h include out to before we include all the perl stuff. That > > > does > > > fix the issue. > > > > We could also move is_valid_ascii somewhere else. It's only > > tangentially related to "wide chars" anyway. > > Given the crazy defines of stuff like free, it seems like a good idea to have > a rule that no headers should be included after plperl.h with > PG_NEED_PERL_XSUB_H defined. It's not like there's not other chances of of > pulling in malloc.h from within pg_wchar.h somehow. > > It's a bit ugly to have the mb/pg_wchar.h in plperl.h instead of > plperl_helpers.h, but ... Greetings, Andres Freund
Re: windows cfbot failing: my_perl
Andres Freund writes: > Tom, Ilmari, you seem to have hacked on this stuff most (not so) recently. Do > you have a better suggestion than moving the mb/pg_wchar.h include out of > plperl_helpers.h as I suggest below? I agree with the conclusion that we'd better #include all our own headers before any of Perl's. No strong opinions about which rearrangement is least ugly --- but let's add some comments about that requirement. regards, tom lane
Re: SQL/JSON features for v15
On 2022-08-26 Fr 16:11, Nikita Glukhov wrote: > > Hi, > > On 26.08.2022 22:25, Andrew Dunstan wrote: >> On 2022-08-24 We 20:05, Nikita Glukhov wrote: >>> v8 - is a highly WIP patch, which I failed to finish today. >>> Even some test cases fail now, and they simply show unfinished >>> things like casts to bytea (they can be simply removed) and missing >>> safe input functions. >> Thanks for your work, please keep going. > I have completed in v9 all the things I previously planned: > > - Added missing safe I/O and type conversion functions for >datetime, float4, varchar, bpchar. This introduces a lot >of boilerplate code for returning errors and also maybe >adds some overhead. > > - Added JSON_QUERY coercion to UTF8 bytea using pg_convert_to(). > > - Added immutability checks that were missed with elimination >of coercion expressions. >Coercions text::datetime, datetime1::datetime2 and even >datetime::text for some datetime types are mutable. >datetime::text can be made immutable by passing ISO date >style into output functions (like in jsonpath). > > - Disabled non-Const expressions in DEFAULT ON EMPTY in non >ERROR ON ERROR case. Non-constant expressions are tried to >evaluate into Const directly inside transformExpr(). >Maybe it would be better to simply remove DEFAULT ON EMPTY. Yes, I think that's what I suggested upthread. I don't think DEFAULT ON EMPTY matters that much, and we can revisit it for release 16. If it's simpler please do it that way. > > > It is possible to easily split this patch into several subpatches, > I will do it if needed. Thanks, probably a good idea but I will start reviewing what you have now. Andres and others please chime in if you can. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
Re: windows cfbot failing: my_perl
On 2022-08-26 Fr 10:47, Andres Freund wrote: > Hi, > > On 2022-08-26 21:39:05 +0700, John Naylor wrote: >> On Fri, Aug 26, 2022 at 9:27 PM Andres Freund wrote: >>> Because perl, extremely unhelpfully, #defines free. Which, not surprisingly, >>> causes issues when including system headers referencing free as well. >>> >>> I don't really see a good solution to this other than hoisting the >>> mb/pg_wchar.h include out to before we include all the perl stuff. That does >>> fix the issue. >> We could also move is_valid_ascii somewhere else. It's only >> tangentially related to "wide chars" anyway. > Given the crazy defines of stuff like free, it seems like a good idea to have > a rule that no headers should be included after plperl.h with > PG_NEED_PERL_XSUB_H defined. It's not like there's not other chances of of > pulling in malloc.h from within pg_wchar.h somehow. > > It's a bit ugly to have the mb/pg_wchar.h in plperl.h instead of > plperl_helpers.h, but ... > It's already included directly in plperl.c, so couldn't we just lift it directly into SPI.xs and Util.xs? cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
Re: windows cfbot failing: my_perl
Hi, On 2022-08-26 17:05:52 -0400, Andrew Dunstan wrote: > On 2022-08-26 Fr 10:47, Andres Freund wrote: > > Given the crazy defines of stuff like free, it seems like a good idea to > > have > > a rule that no headers should be included after plperl.h with > > PG_NEED_PERL_XSUB_H defined. It's not like there's not other chances of of > > pulling in malloc.h from within pg_wchar.h somehow. > > > > It's a bit ugly to have the mb/pg_wchar.h in plperl.h instead of > > plperl_helpers.h, but ... > It's already included directly in plperl.c, so couldn't we just lift it > directly into SPI.xs and Util.xs? I think it'd also be needed in hstore_plperl.c, jsonb_plperl.c. Putting the include in plperl.h would keep that aspect transparent, because plperl_utils.h includes plperl.h. I don't think manually including all dependencies, even if it's just one, in each of the six files currently using plperl_utils.h is a good approach. Greetings, Andres Freund
Re: wal_sync_method=fsync_writethrough
On Sat, Aug 27, 2022 at 12:17 AM Magnus Hagander wrote: > So, I don't know how it works now, but the history at least was this: > it was not about the disk caches, it was about raid controller caches. > Basically, we determined that windows didn't fsync it all the way. But > it would with But if we changed wal_sync_method=fsync to actually > *do* that, then people who had paid big money for raid controllers > with flash or battery backed cache would lose a ton of performance. So > we needed one level that would sync out of the OS but not through the > RAID cache, and another one that would sync it out of the RAID cache > as well. Which would/could be different from the drive caches > themselves, and they often behaved differently. And I think it may > have even been dependent on the individual RAID drivers what the > default would be. Thanks for the background. Yeah, that makes sense to motivate open_datasync for Windows. Not sure what you meant about fsync or meant to write after "would with". It seems like the 2005 discussions were primarily about open_datasync but also had the by-product of introducing the name fsync_writethrough. If I'm reading between the lines[1] correctly, perhaps the logic went like this: 1. We noticed that _commit() AKA FlushFileBuffers() issued SYNCHRONIZE CACHE (or equivalent) on Windows. 2. At that time in history, Linux (and other Unixes) probably did not issue SYNCHRONIZE CACHE when you called fsync()/fdatasync(). 3. We concluded therefore that Windows was strange and we needed to use a different level name for the setting to reflect this extra effect. Now it looks strange: we have both "fsync" and "fsync_writethrough" doing exactly the same thing while vaguely implying otherwise, and the contrast with other operating systems (if I divined that aspect correctly) mostly doesn't apply. How flush commands affect various caches in modern storage stacks is also not really OS-specific AFAIK. (Obviously macOS is a different story...) [1] https://www.postgresql.org/message-id/flat/26109.084860%40sss.pgh.pa.us#e7f8c2e14d76cad76b1857e89c8a6314
Re: Removing unneeded self joins
Hi, For v36-0001-Remove-self-joins.patch : bq removes inner join of plane table to itself plane table -> plain table For relation_has_unique_index_ext(), it seems when extra_clauses is NULL, there is no need to compute `exprs`. Cheers >
Re: windows cfbot failing: my_perl
On Sat, Aug 27, 2022 at 4:15 AM Andres Freund wrote: > > Hi, > > On 2022-08-26 17:05:52 -0400, Andrew Dunstan wrote: > > On 2022-08-26 Fr 10:47, Andres Freund wrote: > > > Given the crazy defines of stuff like free, it seems like a good idea to > > > have > > > a rule that no headers should be included after plperl.h with > > > PG_NEED_PERL_XSUB_H defined. It's not like there's not other chances of > > > of > > > pulling in malloc.h from within pg_wchar.h somehow. > > > > > > It's a bit ugly to have the mb/pg_wchar.h in plperl.h instead of > > > plperl_helpers.h, but ... > > > It's already included directly in plperl.c, so couldn't we just lift it > > directly into SPI.xs and Util.xs? > > I think it'd also be needed in hstore_plperl.c, jsonb_plperl.c. Putting the > include in plperl.h would keep that aspect transparent, because plperl_utils.h > includes plperl.h. Since plperl_helpers.h already includes plperl.h, I'm not sure why both are included everywhere the former is. If .c/.xs files didn't include plperl.h directly, we could keep pg_wchar.h in plperl_helpers.h. Not sure if that's workable or any better... -- John Naylor EDB: http://www.enterprisedb.com
Re: [PATCH] Add native windows on arm64 support
On Thu, Aug 25, 2022 at 08:29:43PM -0700, Andres Freund wrote:
> It's really not great that 7f3e17b4827 disabled randomization without even a
> comment as to why...
This story is on this thread, with some processes not able to start:
https://www.postgresql.org/message-id/BD0D89EC2438455C9DE0DC94D36912F4@maumau
> There actually seems to have been a lot of work in that area. See 617dc6d299c
> / bcbf2346d69 and as well as the retrying in 45e004fb4e39. Those weren't
> prevented by us disabling ASLR.
Indeed. 45e004f looks like the most interesting bit here. FWIW, I
would not mind re-enabling that on HEAD, as of something like the
attached. I have done a dozen of runs without seeing a test failure,
and knowing that we don't support anything older than Win10 makes me
feel safer about this change. Any objections?
--
Michael
From 0fde47c2d4e65bd9f1f5a7b4607f8b8b95162242 Mon Sep 17 00:00:00 2001
From: Michael Paquier
Date: Sat, 27 Aug 2022 10:20:53 +0900
Subject: [PATCH] Re-enable ASLR on Windows
---
src/tools/msvc/MSBuildProject.pm | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/tools/msvc/MSBuildProject.pm b/src/tools/msvc/MSBuildProject.pm
index 62acdda3a1..594729ceb7 100644
--- a/src/tools/msvc/MSBuildProject.pm
+++ b/src/tools/msvc/MSBuildProject.pm
@@ -347,7 +347,6 @@ sub WriteItemDefinitionGroup
.\\$cfgname\\$self->{name}\\$self->{name}.pdb
false
.\\$cfgname\\$self->{name}\\$self->{name}.map
- false
Console
--
2.37.2
signature.asc
Description: PGP signature
Re: Use array as object (src/fe_utils/parallel_slot.c)
On Fri, Aug 26, 2022 at 01:54:26PM -0300, Ranier Vilela wrote: > Is it worth creating a commiffest? Don't think so, but feel free to create one and mark me as committer if you think that's appropriate. I have marked this thread as something to do soon-ishly, but I am being distracted by life this month. -- Michael signature.asc Description: PGP signature
Re: windows cfbot failing: my_perl
John Naylor writes: > On Sat, Aug 27, 2022 at 4:15 AM Andres Freund wrote: >> I think it'd also be needed in hstore_plperl.c, jsonb_plperl.c. Putting the >> include in plperl.h would keep that aspect transparent, because >> plperl_utils.h >> includes plperl.h. > Since plperl_helpers.h already includes plperl.h, I'm not sure why > both are included everywhere the former is. If .c/.xs files didn't > include plperl.h directly, we could keep pg_wchar.h in > plperl_helpers.h. Not sure if that's workable or any better... Maybe we should flush the separate plperl_helpers.h header and just put those static-inline functions in plperl.h. regards, tom lane
Re: [PATCH] Add native windows on arm64 support
Michael Paquier writes: > Indeed. 45e004f looks like the most interesting bit here. FWIW, I > would not mind re-enabling that on HEAD, as of something like the > attached. I have done a dozen of runs without seeing a test failure, > and knowing that we don't support anything older than Win10 makes me > feel safer about this change. Any objections? We're early enough in the v16 cycle to have plenty of time to detect any problems, so I see little reason not to try it. regards, tom lane
Re: windows cfbot failing: my_perl
On Sat, Aug 27, 2022 at 10:02 AM Tom Lane wrote:
>
> John Naylor writes:
> > On Sat, Aug 27, 2022 at 4:15 AM Andres Freund wrote:
> >> I think it'd also be needed in hstore_plperl.c, jsonb_plperl.c. Putting the
> >> include in plperl.h would keep that aspect transparent, because
> >> plperl_utils.h
> >> includes plperl.h.
>
> > Since plperl_helpers.h already includes plperl.h, I'm not sure why
> > both are included everywhere the former is. If .c/.xs files didn't
> > include plperl.h directly, we could keep pg_wchar.h in
> > plperl_helpers.h. Not sure if that's workable or any better...
>
> Maybe we should flush the separate plperl_helpers.h header and just
> put those static-inline functions in plperl.h.
Here's a patch with that idea, not tested on Windows yet.
--
John Naylor
EDB: http://www.enterprisedb.com
From b9fba2229b064ab3d7971917cf9bfc1f95bc2d6f Mon Sep 17 00:00:00 2001
From: John Naylor
Date: Sat, 27 Aug 2022 11:17:36 +0700
Subject: [PATCH v1] Be more careful to avoid including system headers after
perl.h
Commit 121d2d3d70 included simd.h into pg_wchar.h, which lead to perl.h's
free() being redefined, at least on Windows. To fix, move the static
inline function definitions from plperl_helpers.h, into plperl.h, where
we already document the necessary inclusion order. Since those functions
were the only reason for the existence of plperl_helpers.h, remove it.
First reported by Justin Pryzby
Diagnosis by Andres Freund, patch by myself per suggestion from Tom Lane
Discussion: https://www.postgresql.org/message-id/20220826115546.GE2342%40telsasoft.com
---
contrib/hstore_plperl/hstore_plperl.c | 1 -
contrib/jsonb_plperl/jsonb_plperl.c | 1 -
src/pl/plperl/GNUmakefile | 4 +-
src/pl/plperl/SPI.xs | 1 -
src/pl/plperl/Util.xs | 1 -
src/pl/plperl/plperl.c| 2 -
src/pl/plperl/plperl.h| 170 -
src/pl/plperl/plperl_helpers.h| 171 --
8 files changed, 171 insertions(+), 180 deletions(-)
delete mode 100644 src/pl/plperl/plperl_helpers.h
diff --git a/contrib/hstore_plperl/hstore_plperl.c b/contrib/hstore_plperl/hstore_plperl.c
index c72785d99e..4a1629cad5 100644
--- a/contrib/hstore_plperl/hstore_plperl.c
+++ b/contrib/hstore_plperl/hstore_plperl.c
@@ -3,7 +3,6 @@
#include "fmgr.h"
#include "hstore/hstore.h"
#include "plperl.h"
-#include "plperl_helpers.h"
PG_MODULE_MAGIC;
diff --git a/contrib/jsonb_plperl/jsonb_plperl.c b/contrib/jsonb_plperl/jsonb_plperl.c
index 22e90afe1b..2af1e0c02a 100644
--- a/contrib/jsonb_plperl/jsonb_plperl.c
+++ b/contrib/jsonb_plperl/jsonb_plperl.c
@@ -4,7 +4,6 @@
#include "fmgr.h"
#include "plperl.h"
-#include "plperl_helpers.h"
#include "utils/fmgrprotos.h"
#include "utils/jsonb.h"
diff --git a/src/pl/plperl/GNUmakefile b/src/pl/plperl/GNUmakefile
index a2e6410f53..1ebf3c9ba2 100644
--- a/src/pl/plperl/GNUmakefile
+++ b/src/pl/plperl/GNUmakefile
@@ -72,7 +72,7 @@ XSUBPPDIR = $(shell $(PERL) -e 'use List::Util qw(first); print first { -r "$$_/
include $(top_srcdir)/src/Makefile.shlib
-plperl.o: perlchunks.h plperl_opmask.h plperl_helpers.h
+plperl.o: perlchunks.h plperl_opmask.h
plperl_opmask.h: plperl_opmask.pl
@if [ x"$(perl_privlibexp)" = x"" ]; then echo "configure switch --with-perl was not specified."; exit 1; fi
@@ -103,7 +103,7 @@ uninstall: uninstall-lib uninstall-data
install-data: installdirs
$(INSTALL_DATA) $(addprefix $(srcdir)/, $(DATA)) '$(DESTDIR)$(datadir)/extension/'
- $(INSTALL_DATA) $(srcdir)/plperl.h $(srcdir)/ppport.h $(srcdir)/plperl_helpers.h '$(DESTDIR)$(includedir_server)'
+ $(INSTALL_DATA) $(srcdir)/plperl.h $(srcdir)/ppport.h '$(DESTDIR)$(includedir_server)'
uninstall-data:
rm -f $(addprefix '$(DESTDIR)$(datadir)/extension'/, $(notdir $(DATA)))
diff --git a/src/pl/plperl/SPI.xs b/src/pl/plperl/SPI.xs
index b2db3bd694..e81432e634 100644
--- a/src/pl/plperl/SPI.xs
+++ b/src/pl/plperl/SPI.xs
@@ -13,7 +13,6 @@
/* perl stuff */
#define PG_NEED_PERL_XSUB_H
#include "plperl.h"
-#include "plperl_helpers.h"
MODULE = PostgreSQL::InServer::SPI PREFIX = spi_
diff --git a/src/pl/plperl/Util.xs b/src/pl/plperl/Util.xs
index 47eba59415..bb4580ebfa 100644
--- a/src/pl/plperl/Util.xs
+++ b/src/pl/plperl/Util.xs
@@ -20,7 +20,6 @@
/* perl stuff */
#define PG_NEED_PERL_XSUB_H
#include "plperl.h"
-#include "plperl_helpers.h"
static text *
diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c
index af354a68cc..5d192a0ce5 100644
--- a/src/pl/plperl/plperl.c
+++ b/src/pl/plperl/plperl.c
@@ -23,7 +23,6 @@
#include "commands/trigger.h"
#include "executor/spi.h"
#include "funcapi.h"
-#include "mb/pg_wchar.h"
#include "miscadmin.h"
#include "nodes/makefuncs.h"
#include "parser/parse_type.h"
@@ -47,7 +46,6 @@
/* string literal macros defining chunks of perl code */
#include "perlchunks.h"
#include "plperl.h"
-#include "plperl_helpers.h"
/* defines PLPERL_S
Re: Bump MIN_WINNT to 0x0600 (Vista) as minimal runtime in 16~
On Fri, Aug 26, 2022 at 06:26:37AM -0500, Justin Pryzby wrote:
> If I'm not wrong, there's some lingering comments which could be removed since
> 495ed0ef2.
It seems to me that you are right. I have not thought about looking
at references to NT. Good catches!
> src/bin/pg_ctl/pg_ctl.c: * on NT4. That way, we don't break on NT4.
> src/bin/pg_ctl/pg_ctl.c: * On NT4, or any other system not containing the
> required functions, will
> src/bin/pg_ctl/pg_ctl.c: * NT4 doesn't have
> CreateRestrictedToken, so just call ordinary
> src/port/dirmod.c: *Win32 (NT4 and newer).
> src/backend/port/win32/socket.c:/* No error, zero bytes
> (win2000+) or error+WSAEWOULDBLOCK (<=nt4) */
There is also a reference to Nt4 in win32.c, for a comment that is
irrelevant now, so it can be IMO removed.
There may be a point in enforcing CreateProcess() if
CreateRestrictedToken() cannot be loaded, but that would be a security
issue if Windows goes crazy as we should always expect the function,
so this had better return an error.
So, what do you think about the attached?
--
Michael
diff --git a/src/backend/port/win32/socket.c b/src/backend/port/win32/socket.c
index 52944a0d33..130b60af22 100644
--- a/src/backend/port/win32/socket.c
+++ b/src/backend/port/win32/socket.c
@@ -495,7 +495,7 @@ pgwin32_send(SOCKET s, const void *buf, int len, int flags)
return -1;
}
- /* No error, zero bytes (win2000+) or error+WSAEWOULDBLOCK (<=nt4) */
+ /* No error, zero bytes */
if (pgwin32_waitforsinglesocket(s, FD_WRITE | FD_CLOSE, INFINITE) == 0)
return -1;
diff --git a/src/port/dirmod.c b/src/port/dirmod.c
index 2818bfd2e9..ae6301dd6c 100644
--- a/src/port/dirmod.c
+++ b/src/port/dirmod.c
@@ -7,7 +7,7 @@
* Portions Copyright (c) 1994, Regents of the University of California
*
* This includes replacement versions of functions that work on
- * Win32 (NT4 and newer).
+ * Windows.
*
* IDENTIFICATION
* src/port/dirmod.c
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index 73e20081d1..20d2a04b7f 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -1726,9 +1726,7 @@ pgwin32_doRunAsService(void)
/*
* Mingw headers are incomplete, and so are the libraries. So we have to load
- * a whole lot of API functions dynamically. Since we have to do this anyway,
- * also load the couple of functions that *do* exist in mingw headers but not
- * on NT4. That way, we don't break on NT4.
+ * a whole lot of API functions dynamically.
*/
typedef BOOL (WINAPI * __CreateRestrictedToken) (HANDLE, DWORD, DWORD, PSID_AND_ATTRIBUTES, DWORD, PLUID_AND_ATTRIBUTES, DWORD, PSID_AND_ATTRIBUTES, PHANDLE);
typedef BOOL (WINAPI * __IsProcessInJob) (HANDLE, HANDLE, PBOOL);
@@ -1768,9 +1766,6 @@ InheritStdHandles(STARTUPINFO *si)
*
* Returns 0 on success, non-zero on failure, same as CreateProcess().
*
- * On NT4, or any other system not containing the required functions, will
- * launch the process under the current token without doing any modifications.
- *
* NOTE! Job object will only work when running as a service, because it's
* automatically destroyed when pg_ctl exits.
*/
@@ -1815,14 +1810,9 @@ CreateRestrictedProcess(char *cmd, PROCESS_INFORMATION *processInfo, bool as_ser
if (_CreateRestrictedToken == NULL)
{
- /*
- * NT4 doesn't have CreateRestrictedToken, so just call ordinary
- * CreateProcess
- */
- write_stderr(_("%s: WARNING: cannot create restricted tokens on this platform\n"), progname);
- if (Advapi32Handle != NULL)
- FreeLibrary(Advapi32Handle);
- return CreateProcess(NULL, cmd, NULL, NULL, FALSE, 0, NULL, NULL, &si, processInfo);
+ /* Log error if we cannot get the function */
+ write_stderr(_("%s: WARNING: could not locate object function to create restricted token\n"), progname);
+ return 0;
}
/* Open the current token to use as a base for the restricted one */
diff --git a/src/interfaces/libpq/win32.c b/src/interfaces/libpq/win32.c
index e57b602476..447f64c072 100644
--- a/src/interfaces/libpq/win32.c
+++ b/src/interfaces/libpq/win32.c
@@ -271,10 +271,6 @@ struct MessageDLL
* Returns a description of the socket error by first trying
* to find it in the lookup table, and if that fails, tries
* to load any of the winsock dlls to find that message.
- * The DLL thing works from Nt4 (spX ?) up, but some special
- * versions of winsock might have this as well (seen on Win98 SE
- * special install) / Magnus Naeslund (m...@fbab.net)
- *
*/
const char *
signature.asc
Description: PGP signature
Re: windows cfbot failing: my_perl
On Sat, Aug 27, 2022 at 11:20 AM John Naylor wrote: > > Here's a patch with that idea, not tested on Windows yet. Update: I tried taking the CI for a spin, but ran into IT issues with Github when I tried to push my branch to remote. -- John Naylor EDB: http://www.enterprisedb.com
Re: [BUG] Logical replication failure "ERROR: could not map filenode "base/13237/442428" to relation OID" with catalog modifying txns
On Fri, Jul 29, 2022 at 12:15 PM Amit Kapila wrote:
>
> >
> > Yeah, your description makes sense to me. I've also considered how to
> > hit this path but I guess it is never hit. Thinking of it in another
> > way, first of all, at least 2 catalog modifying transactions have to
> > be running while writing a xl_running_xacts. The serialized snapshot
> > that is written when we decode the first xl_running_xact has two
> > transactions. Then, one of them is committed before the second
> > xl_running_xacts. The second serialized snapshot has only one
> > transaction. Then, the transaction is also committed after that. Now,
> > in order to execute the path, we need to start decoding from the first
> > serialized snapshot. However, if we start from there, we cannot decode
> > the full contents of the transaction that was committed later.
> >
>
> I think then we should change this code in the master branch patch
> with an additional comment on the lines of: "Either all the xacts got
> purged or none. It is only possible to partially remove the xids from
> this array if one or more of the xids are still running but not all.
> That can happen if we start decoding from a point (LSN where the
> snapshot state became consistent) where all the xacts in this were
> running and then at least one of those got committed and a few are
> still running. We will never start from such a point because we won't
> move the slot's restart_lsn past the point where the oldest running
> transaction's restart_decoding_lsn is."
>
Unfortunately, this theory doesn't turn out to be true. While
investigating the latest buildfarm failure [1], I see that it is
possible that only part of the xacts in the restored catalog modifying
xacts list needs to be purged. See the attached where I have
demonstrated it via a reproducible test. It seems the point we were
missing was that to start from a point where two or more catalog
modifying were serialized, it requires another open transaction before
both get committed, and then we need the checkpoint or other way to
force running_xacts record in-between the commit of initial two
catalog modifying xacts. There could possibly be other ways as well
but the theory above wasn't correct.
[1] -
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=curculio&dt=2022-08-25%2004%3A15%3A34
--
With Regards,
Amit Kapila.
diff --git a/contrib/test_decoding/expected/catalog_change_snapshot.out b/contrib/test_decoding/expected/catalog_change_snapshot.out
index dc4f9b7..d2a4bdf 100644
--- a/contrib/test_decoding/expected/catalog_change_snapshot.out
+++ b/contrib/test_decoding/expected/catalog_change_snapshot.out
@@ -1,4 +1,4 @@
-Parsed test spec with 2 sessions
+Parsed test spec with 3 sessions
starting permutation: s0_init s0_begin s0_savepoint s0_truncate s1_checkpoint s1_get_changes s0_commit s0_begin s0_insert s1_checkpoint s1_get_changes s0_commit s1_get_changes
step s0_init: SELECT 'init' FROM pg_create_logical_replication_slot('isolation_slot', 'test_decoding');
@@ -42,3 +42,57 @@ COMMIT
stop
(1 row)
+
+starting permutation: s0_init s0_begin s0_truncate s2_begin s2_truncate s1_checkpoint s1_get_changes s0_commit s0_begin s0_insert s1_checkpoint s1_get_changes s2_commit s1_checkpoint s1_get_changes s0_commit s1_get_changes
+step s0_init: SELECT 'init' FROM pg_create_logical_replication_slot('isolation_slot', 'test_decoding');
+?column?
+
+init
+(1 row)
+
+step s0_begin: BEGIN;
+step s0_truncate: TRUNCATE tbl1;
+step s2_begin: BEGIN;
+step s2_truncate: TRUNCATE tbl2;
+step s1_checkpoint: CHECKPOINT;
+step s1_get_changes: SELECT data FROM pg_logical_slot_get_changes('isolation_slot', NULL, NULL, 'skip-empty-xacts', '1', 'include-xids', '0');
+data
+
+(0 rows)
+
+step s0_commit: COMMIT;
+step s0_begin: BEGIN;
+step s0_insert: INSERT INTO tbl1 VALUES (1);
+step s1_checkpoint: CHECKPOINT;
+step s1_get_changes: SELECT data FROM pg_logical_slot_get_changes('isolation_slot', NULL, NULL, 'skip-empty-xacts', '1', 'include-xids', '0');
+data
+---
+BEGIN
+table public.tbl1: TRUNCATE: (no-flags)
+COMMIT
+(3 rows)
+
+step s2_commit: COMMIT;
+step s1_checkpoint: CHECKPOINT;
+step s1_get_changes: SELECT data FROM pg_logical_slot_get_changes('isolation_slot', NULL, NULL, 'skip-empty-xacts', '1', 'include-xids', '0');
+data
+---
+BEGIN
+table public.tbl2: TRUNCATE: (no-flags)
+COMMIT
+(3 rows)
+
+step s0_commit: COMMIT;
+step s1_get_changes: SELECT data FROM pg_logical_slot_get_changes('isolation_slot', NULL, NULL, 'skip-empty-xacts', '1', 'include-xids', '0');
+data
+-
+BEGIN
Re: use ARM intrinsics in pg_lfind32() where available
On Sat, Aug 27, 2022 at 1:24 AM Nathan Bossart wrote: > > Here is a rebased patch set that applies to HEAD. 0001: #define USE_NO_SIMD typedef uint64 Vector8; +typedef uint64 Vector32; #endif I don't forsee any use of emulating vector registers with uint64 if they only hold two ints. I wonder if it'd be better if all vector32 functions were guarded with #ifndef NO_USE_SIMD. (I wonder if declarations without definitions cause warnings...) + * NB: This function assumes that each lane in the given vector either has all + * bits set or all bits zeroed, as it is mainly intended for use with + * operations that produce such vectors (e.g., vector32_eq()). If this + * assumption is not true, this function's behavior is undefined. + */ Hmm? Also, is_highbit_set() already has uses same intrinsic and has the same intended effect, since we only care about the boolean result. 0002: -#elif defined(USE_SSE2) +#elif defined(USE_SSE2) || defined(USE_NEON) I think we can just say #else. -#if defined(USE_SSE2) - __m128i sub; +#ifndef USE_NO_SIMD + Vector8 sub; +#elif defined(USE_NEON) + + /* use the same approach as the USE_SSE2 block above */ + sub = vqsubq_u8(v, vector8_broadcast(c)); + result = vector8_has_zero(sub); I think we should invent a helper that does saturating subtraction and call that, inlining the sub var so we don't need to mess with it further. Otherwise seems fine. -- John Naylor EDB: http://www.enterprisedb.com
