On Thu, Mar 18, 2021 at 09:38:05AM -0700, Peter Geoghegan wrote:
> Were you going to take care of this, Michael?
Yes, I was waiting for Sawada-san to send an updated version, which he
just did.
--
Michael
signature.asc
Description: PGP signature
On Thu, Mar 18, 2021 at 07:45:36AM +, gkokola...@pm.me wrote:
> It seems helpful. Thank you.
Thanks, applied then.
--
Michael
signature.asc
Description: PGP signature
the parallel case and the ones in local memory,
but what we have here looks enough to me so we could figure out that
as a future step.
Sawada-san, what do you think? Attached is the patch I have finished
with.
--
Michael
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/h
On Thu, Mar 18, 2021 at 05:14:24PM +0900, Michael Paquier wrote:
> Looking at 0001, I am not much a fan of relying on the position of the
> matching pattern in the log file. Instead of relying on the logging
> collector and one single file, why not just changing the generation of
>
In other words, vacrelstats->indstats[i] is
> never updated if the corresponding index supports parallel indexes. I
> think this is not relevant with the change that we'd like to do here
> (i.e., passing indnum down).
Yeah, that looks like just some over-engineering design on my
an
overlap. That's one problem that SQL pattern checks had to deal with
in the past. Thoughts?
--
Michael
signature.asc
Description: PGP signature
On Sat, Mar 20, 2021 at 05:37:47PM +0900, Michael Paquier wrote:
> It seems to me that this would make the tests faster, that the test
> would not need to wait for the logging collector and that the code
> could just use slurp_file($node->logfile) to get the data it wants to
> ch
On Thu, Mar 18, 2021 at 12:56:12PM +0900, Michael Paquier wrote:
> I was looking at uses of ThisTimeLineID in the wild, and could not
> find it getting checked or used actually in backend-side code that
> involved the WAL reader facility. Even if it brings confidence, it
> does not
looks good to me and the kerberos regression tests pass with it.
Thanks Stephen and Bharath for looking at it! I have applied that
now.
--
Michael
signature.asc
Description: PGP signature
acked here:
https://wiki.postgresql.org/wiki/PostgreSQL_14_Open_Items
For the PostgreSQL 14 release, the release management team is composed
of:
Peter Geoghegan
Andrew Dunstan
Michael Paquier
For the time being, if you have any questions about the process,
please feel free to email any member of th
On Thu, Mar 18, 2021 at 12:01:40PM +0900, Michael Paquier wrote:
> Yep, that's actually something I wrote for my own setups, with
> log_checkpoints enabled to catch all concurrent checkpoint activity
> and some LOGs. Still no luck unfortunately :(
The various reporters had more lu
earch-forward functionality that was
> spread across multiple places before. So I've done that too.
I have briefly looked at 0002 (0001 in the attached set), and it seems
sane to me. I still need to look at 0003 (well, now 0002) in details,
which is very sensible as one mistake would li
another day, if there is any need for such a
move.
To keep it short. Sold.
--
Michael
signature.asc
Description: PGP signature
nd_checks_all() and such.
--
Michael
signature.asc
Description: PGP signature
that one committed
> from that thread, and then rebase using that.
Independent and useful pieces could just be extracted and applied
separately where it makes sense. I am not sure if that's the case
here, so I'll do a patch_to_review++.
--
Michael
signature.asc
Description: PGP signature
pointer.c:double CheckPointCompletionTarget = 0.5;
--
Michael
signature.asc
Description: PGP signature
per than it looks and that more
code paths could be involved.
It takes some time to initialize a cluster under CLOBBER_CACHE_ALWAYS,
but the test is quick enough to reproduce. It would be good to bisect
the origin point here as a first step.
--
Michael
signature.asc
Description: PGP signature
On Tue, Mar 23, 2021 at 04:12:01PM +0900, Michael Paquier wrote:
> It takes some time to initialize a cluster under CLOBBER_CACHE_ALWAYS,
> but the test is quick enough to reproduce. It would be good to bisect
> the origin point here as a first step.
One bisect later, the winner i
mmand_checks_all
> + issues_sql_like run_log pg_recvlogical_upto
> +);
No actual objections here, but it would be easy to miss the addition
of a new routine. Would an exclusion filter be more adapted, aka
override everything except get_new_node() and info()?
--
Michael
signature.asc
Description: PGP signature
if
> this is a supported use case for NSS?
Are you referring to the case of threading here? This should be a
supported case, as threads created by an application through libpq
could perfectly use completely different connection strings.
--
Michael
signature.asc
Description: PGP signature
to pg_regress to control if this cleanup code is triggered or
not, say something like --no-tablespace-cleanup? Then, you could just
pass down the option by yourself before creating your tablespace path
as you wish.
--
Michael
signature.asc
Description: PGP signature
long as you add the new event
at the end of the existing enum lists, but let's keep the wait events
ordered on HEAD.
--
Michael
signature.asc
Description: PGP signature
;
+ mkdir("$pgdata/root+client-crldir");
+ copy_files("ssl/root+client-crldir/*",
"$pgdata/root+client-crldir/");
+ }
+ elsif (defined($nss))
+ {
+ RecursiveCopy::copypath("ssl/nss", $pgdata . "/nss") if -e
"ssl/nss";
+ }
This had better be in its own callback, for example.
--
Michael
signature.asc
Description: PGP signature
+clientCertCN,
> +clientCertDN
> +}ClientCertName;
Missing some indentation stuff here.
> +# correct client cert using whole DN
> +my $dn_connstr = $common_connstr;
> +$dn_connstr =~ s/certdb/certdb_dn/;
I would use a separate variable rather than enforcing an update of the
existing $common_connstr created a couple of lines above.
> +# same thing but with a regex
> +$dn_connstr =~ s/certdb_dn/certdb_dn_re/;
Same here. This depends on the last variable assignment, which itself
depends on the assignment of $common_connstr.
--
Michael
signature.asc
Description: PGP signature
x27;mkdir @testtablespace@';
>> CREATE TABLESPACE regress_tblspace LOCATION '@testtablespace@';
>
> Would that work on Windows?
I doubt that all the Windows environments would be able to get their
hands on that. And I am not sure either that this would work when it
c
set them with
> { bindir => ..., libdir => ...} but I doubt it'll ever be necessary.
This would imply an installation with some fancy --bindir or --libdir
specified in ./configure. Never say never, but I also think that what
has been committed is fine. And the result is simple, tha
in pg_stat_activity or
to allow extensions to access this information. I am actually in
favor of keeping this information in the Port with those pluggability
reasons. How do others feel about that?
--
Michael
signature.asc
Description: PGP signature
ings a bit to adapt with
some of the context of the code, and applied it. Thanks!
--
Michael
signature.asc
Description: PGP signature
On top of what's
proposed, would it make sense to have a second logdetail for the case
of a mock authentication? We don't log that yet, so I guess that it
could be useful for audit purposes?
--
Michael
signature.asc
Description: PGP signature
On Thu, Mar 25, 2021 at 06:51:22PM +, Jacob Champion wrote:
> On Thu, 2021-03-25 at 14:41 +0900, Michael Paquier wrote:
>> + port->authn_id = MemoryContextStrdup(TopMemoryContext, id);
>> It may not be obvious that all the field is copied to TopMemoryContext
>> becaus
7;s proposed upthread to
outline that this is a client proof mismatch.
--
Michael
signature.asc
Description: PGP signature
on failure,
so I would check after <= 0 here.
++ if (port->hba->clientcertname == clientCertDN)
++ {
++ ereport(LOG,
May be better to use a switch() here as well.
It looks like this patch misses src/test/ssl/ssl/cli
On Thu, Mar 25, 2021 at 09:23:22AM +0100, Peter Eisentraut wrote:
> On 25.03.21 04:47, Michael Paquier wrote:
>> This would imply an installation with some fancy --bindir or --libdir
>> specified in ./configure. Never say never, but I also think that what
>> has been committ
t that's rather limited.
Generating WAL for the truncation of foreign tables sounds also like a
strange concept to me. I think that you should just make the patch
work so as the truncation is passed down to the FDW that decides what
it needs to do with it, and do nothing more than that.
--
Michael
"on Windows"?
--
Michael
signature.asc
Description: PGP signature
tter, without actually being meaningfully better.
Agreed. Still, wouldn't it be better to avoid such configurations and
protect a bit things with a check on the new value?
--
Michael
signature.asc
Description: PGP signature
the authenticated log is logged before the authorized log, with user
name map checks in-between for some of the auth methods. HEAD refers
to the existing authorized log as "authentication" in the logs, while
you correct that.
--
Michael
signature.asc
Description: PGP signature
thout
some explanation with the name maps? It seems that there is no place
in the existing docs where this difference is explained. I am
wondering if it would be better to not change this paragraph, or
reword it slightly to outline that this may cause more than one log
entry, say:
"Causes each attempted connection to the server, and each
authentication activity to be logged."
--
Michael
signature.asc
Description: PGP signature
e here. If you can get a
patch, please feel free to add it to the next commit fest, for
Postgres 15:
https://commitfest.postgresql.org/33/
--
Michael
signature.asc
Description: PGP signature
On Mon, Mar 29, 2021 at 10:57:00AM +0900, Michael Paquier wrote:
> + switch (port->hba->clientcertname)
> + {
> + case clientCertDN:
> + peer_username = port->peer_dn;
> + break;
> + default:
> + peer_username = port-&g
sts (used the trick with plpgsql for the
latter to avoid the dump of the extension test_pg_dump or any data
related to it).
--
Michael
From ca1388a4c4441f15c02d332b8a9996408a90662c Mon Sep 17 00:00:00 2001
From: Michael Paquier
Date: Tue, 30 Mar 2021 12:01:13 +0900
Subject: [PATCH] Add support fo
ll fix in a couple
of minutes the whole set.
--
Michael
signature.asc
Description: PGP signature
tice this one. Let's leave that alone then,
except if there are more people in favor of fixing the whole set.
--
Michael
signature.asc
Description: PGP signature
does mostly that, but it seems to me that we
should integrate better with PostgresNode to manage the backend node,
no?
> Incidentally, I'm not sure why we need to break SSLServer into
> SSL::Server - are we expecting to create other children of the SSL
> namespace?
Agreed.
--
Michael
signature.asc
Description: PGP signature
say I did my best to create the repoduction
> though -- the explanation above seems to be enough.
Why not just using DISABLE_PAGE_SKIPPING instead here?
--
Michael
signature.asc
Description: PGP signature
On Tue, Mar 30, 2021 at 03:50:28PM +0900, Michael Paquier wrote:
> The test_*() ones are just wrappers for psql able to use a customized
> connection string. It seems to me that it would make sense to move
> those two into PostgresNode::psql itself and extend it to be able to
> h
from a technical POV it's useful as it closes a gap between
pg_dumpall -g and pg_dump -Fc $DATABASE in my opinion, without having to
potentially schema-dump and filter out a large number of database
objects.
Anybody else have some opinions on what to call this best? Maybe just a
short option and some
uld return NULL since it's not inside nor outside the
> polygon, but I'm fine with false.
Yeah, this is trying to make an undefined point fit into a box that
has a definition, so "false" does not make sense to me either here as
it implies that the point exists? NULL
On Tue, Mar 30, 2021 at 08:30:15PM +0800, Julien Rouhaud wrote:
> I just noticed that the comment for CreateStmt.inhRelations says that it's a
> List of inhRelation, which hasn't been the case for a very long time.
Thanks, applied.
--
Michael
signature.asc
Description: PGP signature
On Tue, Mar 30, 2021 at 12:02:45PM +0900, Michael Paquier wrote:
> Okay. So I have looked at that stuff in details, and after fixing
> all the issues reported upthread in the code, docs and tests I am
> finishing with the attached. The tests have been moved out of
> src/bin/pg_dump/
s where we could do
more clarification. For instance, reindex.sgml mentions twice an
exclusive lock but that should be an access exclusive lock. To be
exact, I can spot 27 places under doc/ that could be improved. Such
changes depend on the surrounding context, of course.
--
Michael
signature.asc
Description: PGP signature
ng tomorrow.
There is already TestLib::check_pg_config(). Shouldn't you leverage
that with PG_VERSION_NUM or equivalent?
--
Michael
signature.asc
Description: PGP signature
;> the same line broken pattern of the others the concat looks a bit weird as a
>> result.
>
> +1 for using a single scalar.
Agreed. I posted things this way to make a lookup at the diffs easier
for the eye, but that was not intended for the final patch.
--
Michael
signature.asc
Description: PGP signature
ll the connection strings and with a proper indentation, I finish
with the attached. Does that look fine?
--
Michael
diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm
index 1e96357d7e..d6e10544bb 100644
--- a/src/test/perl/PostgresNode.pm
+++ b/src/test/perl/PostgresNode.pm
On Tue, Mar 30, 2021 at 11:06:55PM -0300, Alvaro Herrera wrote:
> On 2021-Mar-31, Michael Paquier wrote:
>> There is already TestLib::check_pg_config(). Shouldn't you leverage
>> that with PG_VERSION_NUM or equivalent?
>
> hmm, I wonder if we shouldn't take the
apping probably belongs in the
> auth method documentation, and this patch doesn't change any authn/z
> behavior.)
>
> v13 also incorporates the latest SSL cert changes, so it's just a
> single patch now. Tests now cover the CN and DN clientname modes. I
> have not
just that, but if
you feel that this is better, of course feel free.
--
Michael
signature.asc
Description: PGP signature
Attached is an updated patch, with a couple of comments tweaks, the
reworked tests and an indentation done.
--
Michael
From 0f308c892cb4bb120572e996b1eb612682134c03 Mon Sep 17 00:00:00 2001
From: Michael Paquier
Date: Wed, 31 Mar 2021 16:05:33 +0900
Subject: [PATCH v15] Log authenticated identity from
working, and must be written without modification.
> + vital to the backup working and must be written byte for byte without
> + modification, which may require opening the file in binary mode.
Okay, that sounds good to me.
--
Michael
signature.asc
Description: PGP signature
On Wed, Mar 31, 2021 at 10:43:00AM +0900, Michael Paquier wrote:
> Jacob has just raised this as an issue for an integration with NLS,
> because it may be possible that things fail with "SSL error" but a
> different error pattern, causing false positives:
> https://www.pos
On Wed, Mar 31, 2021 at 04:42:32PM +0900, Michael Paquier wrote:
> Attached is an updated patch, with a couple of comments tweaks, the
> reworked tests and an indentation done.
Jacob has mentioned me that v15 has some false positives in the SSL
tests, as we may catch in the backend logs pa
one
extra place where we just check for a FATAL, where the trust
authentication failed after a CN mismatch.
Thoughts?
--
Michael
diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
index b1a63f279c..394d221ada 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/
bility between the two flavors. And does libpq already have some
> notion of a "main thread" that I'm missing?
Nope as far as I recall. With OpenSSL, the initialization of the SSL
mutex lock and the crypto callback initialization is done by the first
thread in.
--
Michael
signature.asc
Description: PGP signature
actually cause a diff and
it has a small chance to happen in practice.
It would be good to add a comment explaining why the options are
added (aka just don't skip any pages).
--
Michael
signature.asc
Description: PGP signature
lock, and I
have tweaked a bit the wording of pgrowlocks.sgml.
--
Michael
signature.asc
Description: PGP signature
On Thu, Apr 01, 2021 at 10:58:25AM +0300, Arseny Sher wrote:
> How about the attached?
Sounds fine to me. Sawada-san?
--
Michael
signature.asc
Description: PGP signature
s to be
> deleted now or if you are supposed to add yourself as a committer.
Thanks, I did not notice that. The entry has been updated as it
should.
--
Michael
signature.asc
Description: PGP signature
On Thu, Apr 01, 2021 at 10:54:29PM +0900, Masahiko Sawada wrote:
> Looks good to me too.
Okay, applied and back-patched down to 12 then.
--
Michael
signature.asc
Description: PGP signature
On Fri, Apr 02, 2021 at 12:03:21AM +, Jacob Champion wrote:
> On Thu, 2021-04-01 at 10:21 +0900, Michael Paquier wrote:
> > This stuff can take advantage of 0d1a3343, and I
> > think that we should make the kerberos, ldap, authentication and SSL
> > test suites just
On Thu, Apr 01, 2021 at 09:56:02AM +0900, Michael Paquier wrote:
> Okay, that sounds good to me.
Applied and backpatched then as 6fb66c26 & co.
--
Michael
signature.asc
Description: PGP signature
ave not looked at the patch yet, though. Fujii-san,
are you planning to take care of that? That was my stuff originally,
so I am fine to look at it. But not now, on a Friday afternoon :)
--
Michael
signature.asc
Description: PGP signature
On Fri, Apr 02, 2021 at 06:03:21PM +0900, Fujii Masao wrote:
> On 2021/04/02 16:47, Michael Paquier wrote:
>> I have not looked
>> in details and have not looked at the patch yet, though. Fujii-san,
>> are you planning to take care of that?
>
> Yes, I will. Thanks
On Mon, Feb 15, 2021 at 08:25:27PM +0900, Michael Paquier wrote:
> Again a new rebase, giving v5:
> - Fixed the APIs to return -1 if the caller gives NULL in input, to be
> consistent with cryptohash.
> - Added a length argument to pg_hmac_final(), wiht sanity checks.
So, this pa
On Fri, Apr 02, 2021 at 10:46:16PM +0200, Joel Jacobson wrote:
> Ascii elephant in example by Michael Paquier [1], with ANSI colors added by
> me.
>
> [1]
> https://www.postgresql.org/message-id/CAB7nPqRaacwcaANOYY3Hxd3T0No5RdZXyqM5HB6fta%2BCoDLOEg%40mail.gmail.com
The cred
ave used for some of my tests to validate the implementations.
This uses some result samples one can find on Wikipedia at [1], for
instance.
[1]: https://en.wikipedia.org/wiki/HMAC
--
Michael
hmac_funcs.tar.gz
Description: application/gzip
signature.asc
Description: PGP signature
On Fri, Apr 02, 2021 at 01:45:31PM +0900, Michael Paquier wrote:
> As a whole, this is a consolidation of its own, so let's apply this
> part first.
Slight rebase for this one to take care of the updates with the SSL
error messages.
--
Michael
From 01e836535119dcd5a69ce54e1c86ae51bfba4
On Sun, Apr 04, 2021 at 10:13:26AM +0530, Bharath Rupireddy wrote:
> +1 to remove it and the patch LGTM.
Indeed, there is no point to keep that around. I'll go clean up that
as you propose.
--
Michael
signature.asc
Description: PGP signature
On Thu, Apr 01, 2021 at 11:59:15AM +0900, Michael Paquier wrote:
> Please find attached a patch to tighten a bit all that. The errors
> produced by OpenSSL down to 1.0.1 are the same. I have noticed one
> extra place where we just check for a FATAL, where the trust
> authentication
On Sun, Apr 04, 2021 at 03:49:35PM +0300, Anton Voloshin wrote:
> just a quick patch for a single-letter typo in a comment
> in src/backend/commands/collationcmds.c
> ...
Thanks, fixed. This came from 51e225d.
--
Michael
signature.asc
Description: PGP signature
On Sat, Apr 03, 2021 at 09:30:25PM +0900, Michael Paquier wrote:
> Slight rebase for this one to take care of the updates with the SSL
> error messages.
I have been looking again at that and applied it as c50624cd after
some slight modifications. Attached is the main, refactored, patc
This removal was done as %params was
passed down directly as-is to PostgresNode::psql, but that's not the
case anymore.
--
Michael
signature.asc
Description: PGP signature
an issue with the
reference count of the TupleDesc here, your test case increments two
times a TupleDesc for this custom type in a portal, and tries to
decrement it three times, causing what looks like a leak.
--
Michael
signature.asc
Description: PGP signature
is just some duplicated code with no extra effect. I
have no objection to simplify a bit the whole on readability and
consistency grounds (will do so tomorrow), including the removal of
the commented-out memset call in gistinitpage, but this is not
something that should be backpatched.
--
Michael
s
se LIMIT TO for this
purpose looks sensible from here, and I agree that it can have its
uses. So what you have here LGTM.
--
Michael
signature.asc
Description: PGP signature
Hi,
Am Montag, den 05.04.2021, 14:33 -0400 schrieb Stephen Frost:
> * Michael Banck (michael.ba...@credativ.de) wrote:
> > Am Montag, den 08.03.2021, 20:54 +0500 schrieb Ibrar Ahmed:
> > > On Thu, Dec 31, 2020 at 6:16 PM Michael Banck
> > > wrote:
> > > &
On Tue, Apr 06, 2021 at 06:31:16PM +, Jacob Champion wrote:
> On Tue, 2021-04-06 at 14:15 +0900, Michael Paquier wrote:
> > Hmm. You are making a good point here, but is that really the best
> > thing we can do? We lose the context of the authentication type being
>
h /*
> p->pd_flags = 0;done by above MemSet */.
Sure, but this one does not hurt much either as-is, so I have left it
out, and applied the rest.
--
Michael
signature.asc
Description: PGP signature
an administrator may wish to set BYPASSRLS on roles
> + which this role is GRANTed to.
> +
Shouldn't those "SELECT", "INSERT" etc. be wrapped in tags?
Michael
--
Michael Banck
Projektleiter / Senior Berater
Tel.: +49 2166 9901-171
Fax: +49
ink that it will remain around for some time. Keeping
around the code necessary to silence WITH OIDS has no real maintenance
cost, and removing it could easily break applications. So there is
little gain in cleaning up that, and a lot of potential loss for
users.
--
Michael
signature.asc
Description: PGP signature
Hi,
Am Dienstag, den 06.04.2021, 15:37 +0200 schrieb Michael Banck:
> Am Montag, den 05.04.2021, 14:33 -0400 schrieb Stephen Frost:
> > Should drop the 'DEFAULT_' to match the others since the rename to
> > 'predefined' roles went in.
>
> Right, will sen
ut that also highlights -- shouldn't that function then also be made
> to use hba_authname(), and the assert moved into the function? That
> seems like the cleanest way?
Good idea, that's much cleaner this way. Do you like the attached?
--
Michael
diff --git a/src/include/libpq/hba.h b/s
tially
> more useful for the future.
Missed it, sorry about that. Using UserAuth as argument is fine by
me. If you wish to apply that, please feel free. I am fine to do
that myself, but that will have to wait until tomorrow my time.
--
Michael
signature.asc
Description: PGP signature
On Wed, Apr 07, 2021 at 02:20:25PM +0200, Magnus Hagander wrote:
> Ok, I'll go ahead and push it. Thanks for confirming the fix!
Cool. Thanks!
--
Michael
signature.asc
Description: PGP signature
x27;, Storage Parameters
> section does not mention it as a parameter. The code is fine as is.
But I agree with letting what we have here as it is, per the same
argument of upthread that this could just break stuff for free, and
that's not a maintenance burden either.
--
Michael
signature.asc
Description: PGP signature
On Wed, Apr 07, 2021 at 07:42:11PM -0700, Noah Misch wrote:
> I think this is a bug in $SUBJECT.
Sorry for the late reply. I intend to answer to that and this is
registered as an open item, but I got busy with some other things.
--
Michael
signature.asc
Description: PGP signature
on with the concept of what should be the query string
when it comes to prosqlbody with a parallel run, as it replaces prosrc
in some cases where the function uses SQL as language. If the
buildfarm cannot be put back to green, could it be possible to revert
this patch?
--
Michael
signature.asc
Descr
r than commenting it out, as one could think to remove it, but I
think that it can be important in terms of code comprehension when
reading the area. So I quite like what 96ef3b8 has undone for
pd_flags, but not much what cc59049 did back in 2007. That's a matter
of taste, really.
--
Michael
signature.asc
Description: PGP signature
in terms of consistency,
and that's my impression when I scanned the parallel execution code,
and I don't really get why SQL function bodies should not bind by this
rule. Would people object if I add an open item to track that?
--
Michael
signature.asc
Description: PGP signature
On Wed, Mar 17, 2021 at 02:35:28PM +0900, Michael Paquier wrote:
> So we would likely want a separate function. Another possibility,
> which I find tempting, would be to push down the calculation logic
> relying on physical files down to the table AM themselves with a new
> dedica
On Mon, Apr 05, 2021 at 11:12:22AM +0900, Michael Paquier wrote:
> Please find an updated set, v35, attached, and my apologies for
> breaking again your patch set. While testing this patch set and
> adjusting the SSL tests with HEAD, I have noticed what looks like a
> bug with the DN
801 - 900 of 11799 matches
Mail list logo
