hink that just using LW_EXCLUSIVE for the CIC change would have been
fine enough first, after evaluating the performance impact. We could
evaluate if it is possible to lower the ProcArrayLock acquisition in
those code paths on a separate thread.
--
Michael
signature.asc
Description: PGP signature
.
> BTW, I think there would be an easy win in the hashing codepath, by changing
> to a cheaper hash function. Currently, with or without this patch, we use
> hash_any(). Changing that to murmurhash32() or something similar would be a
> drop-in replacement.
Good idea.
--
Michael
si
x27;s original
>> formulation from commit acac68b2bca, which had this message as DEBUG2
>> without any string formatting.
>
> I don't really have an opinion on this.
That still looks useful for debugging, so DEBUG1 sounds fine to me.
--
Michael
signature.asc
Description: PGP signature
d the attached seems more intuitive to me.
There is more code removed, but not that much either.
--
Michael
From fac12471a159c50d2de7bcf4da9fb2951facda4e Mon Sep 17 00:00:00 2001
From: Michael Paquier
Date: Thu, 19 Nov 2020 12:33:28 +0900
Subject: [PATCH v3] Remove ability to choose randomness source
On Thu, Nov 19, 2020 at 12:13:44PM +0900, Michael Paquier wrote:
> That still looks useful for debugging, so DEBUG1 sounds fine to me.
By the way, it strikes me that you could just do nothing as long as
(log_min_messages > DEBUG1), so you could encapsulate most of the
logic that plays wi
ms overkill given how tiny they are.
If you reorganize the code this way, I think that make coverage
(genhtml mainly) would complain because the same function is defined
multiple times. I have fallen in this trap recently, with 2771fcee.
--
Michael
signature.asc
Description: PGP signature
ore implementations added (HMAC and MD5
still need to be added on top of SHA2), and it messes up with the
existing copyright entries.
So splitting and just including them sounds like the cleanest option
of the set.
--
Michael
signature.asc
Description: PGP signature
run, there are two inconsistent
diffs. Looks fine except for those nits.
--
Michael
signature.asc
Description: PGP signature
be removed, and the tests of matview.sql
and select_into.sql would need some cleanup. We could keep around
some scenarios with some follow-up INSERT queries after the initial
creation.
--
Michael
signature.asc
Description: PGP signature
so let's use those
macros. Others may have some comments here, so let's wait a bit
first.
--
Michael
signature.asc
Description: PGP signature
fference in runtime for EXPLAIN in this case
is quite something.
--
Michael
signature.asc
Description: PGP signature
considered as correct if the random data
passes the pd_upper and pg_lsn checks :/
--
Michael
signature.asc
Description: PGP signature
while attempting to guess the end of WAL
ahead of time. Another thing: could it be useful to have an extra
mode to begin streaming without waiting for consistency to finish?
--
Michael
signature.asc
Description: PGP signature
it seems unproven.
Yeah, I am not 100% sure that there are no code paths calling
currtid2(), and the ODBC is too obscure to me to get to a clear
conclusion. currtid() though, is a different deal thanks to
RETURNING. What about cutting the cake in two and just remove
currtid() then?
--
Michae
On Fri, Nov 20, 2020 at 11:17:44PM +0100, Daniel Gustafsson wrote:
> On 20 Nov 2020, at 01:33, Michael Paquier wrote:
>> knowing that we still need to deal with the OOM failure handling
>> and pass down the error to the callers playing with SHA2, it feels
>> like the most con
can think as being really
reliable is based on two facts:
- Do a check only on pd_checksums, as that validates the full contents
of the page.
- When doing a retry, make sure that there is no concurrent I/O
activity in the shared buffers. This requires an API we don't have
yet.
--
Michael
signature.asc
Description: PGP signature
On Fri, Nov 20, 2020 at 09:50:08PM -0500, Tom Lane wrote:
> Michael Paquier writes:
>> What about cutting the cake in two and just remove
>> currtid() then?
>
> +1. That'd still let us get rid of setLastTid() which is
> the ugliest part of the thing, IMO.
Indee
ith this patch, reordered the
tests and applied the patch.
--
Michael
signature.asc
Description: PGP signature
roved ease-of-use.
So, what you are basically saying is to switch currtid_byreloid() to
become a function local to tid.c. And then have just
currtid_byrelname() and currtid_for_view() call that, right?
--
Michael
signature.asc
Description: PGP signature
On Sat, Nov 21, 2020 at 09:39:28PM -0500, Tom Lane wrote:
> Michael Paquier writes:
>> So, what you are basically saying is to switch currtid_byreloid() to
>> become a function local to tid.c. And then have just
>> currtid_byrelname() and currtid_for_view() call that, r
sum on a page we plucked out of shared buffers since we only
> calculate the checksum when we go to write the page out.
A LSN-based check makes the thing tricky. How do you make sure that
pd_lsn is not itself broken? It could be perfectly possible that a
random on-disk corruption makes p
e possibility of incorrect results, confusing users as well
as people looking at such reports.
>> To sum up, I agree with your proposal to reread the page and rely on
>> ascending LSNs. Can you submit a patch?
>
> Probably would make sense to give Michael an opportunity to comment
her analysis, it is
> never an appropriate action with any Unix-domain socket (because with file
> system namespace sockets, you never get an EADDRINUSE, so it's dead code).
> So my proposal here is to just delete that line from the hint and leave the
> rest the same.
Reading again
On Mon, Nov 23, 2020 at 06:13:17PM -0500, Tom Lane wrote:
> Alvaro Herrera writes:
>> Please feel free to go ahead, including the change to ProcSleep.
>
> Will do.
Thank you both for 450c823 and 789b938.
--
Michael
signature.asc
Description: PGP signature
On Fri, Nov 20, 2020 at 03:25:50PM +0900, Michael Paquier wrote:
> I agree that this makes this code a bit cleaner, so let's use those
> macros. Others may have some comments here, so let's wait a bit
> first.
Got this one committed as of d03d754.
--
Michael
signature.a
te to begin with? get_raw_page() uses
ReadBufferExtended() which gives some level of protection already, so
shouldn't it be better to return an ERROR with ERRCODE_DATA_CORRUPTED
and the block involved?
--
Michael
signature.asc
Description: PGP signature
On Sat, Nov 21, 2020 at 10:19:42AM +0900, Michael Paquier wrote:
> What you meant and what I meant was slightly different here. I meant
> publishing a header in src/include/common/ that would get installed,
> and I'd rather avoid that. And you mean to have the header for local
&g
On Sun, Nov 22, 2020 at 08:11:21PM +0900, Michael Paquier wrote:
> Okay, here you go with the attached. If there are any other comments,
> please feel free.
Hearing nothing, applied this one after going through the ODBC driver
code again this morning. Compatibility is exactly the sa
r documentation.
--
Michael
signature.asc
Description: PGP signature
.
I can see the use case for IMMEDIATE, but I fail to see the use cases
for WAIT and FORCE. CHECKPOINT_FORCE is internally implied for the
end-of-recovery and shutdown checkpoints. WAIT could be a dangerous
thing if disabled, as clients could pile up requests to the
checkpointer for no real purpose.
--
Michael
signature.asc
Description: PGP signature
ribble on */
+ cached_tuple = SearchSysCache1(DATABASEOID, ObjectIdGetDatum(MyDatabaseId));
+ cached_dbform = (Form_pg_database) GETSTRUCT(cached_tuple);
Er, shouldn't we *not* use the system cache at all here then? Or am I
missing something obvious? Please see the attached, that is more
si
r future bugs if
the materialization logic changes. In short, I am surprised that this
calculation is not directly part of TupleTableSlotOps. What we'd want
is to get this information depending on the slot type dealt with, and
with your patch you would miss to handle any new slot type
introduce
27;t show directly a reference to them. FWIW, I
agree with Peter that it would have been nice to keep around the 9.5
hint, and I would wish that the last one remains around. I agree,
however, that there is value in having a switch that suppresses them.
>> would be, though. Maybe
ign based on the registration of a custom
procsignal done at shmem init time would be much more flexible. You
need one registration API and something to get an ID associated to the
custom entry, and that's roughly what Teodor tells upthread.
This needs more work, so I am marking i
? The blue lines
look like they are missing some measurements in-between, so it is hard
to tell if this is an actual improvement or just some lack of data.
--
Michael
signature.asc
Description: PGP signature
hod=hash --table-am=heap -i --partitions 2
This makes the patch trickier than it looks as the USING clause is
located between PARTITION BY and WITH. Also, partitioned tables
cannot use the USING clause so you need to be careful that
createPartitions() also uses the correct table AM.
This stu
On Thu, Nov 19, 2020 at 06:40:19PM +0800, Craig Ringer wrote:
> [off-list for now]
Looks like you have missed something here.
--
Michael
signature.asc
Description: PGP signature
ething corrupted? Init forks for
example are made of up to 2 blocks, so the window would get short for
at least those. There are many instances with tables that have few
pages as well.
--
Michael
signature.asc
Description: PGP signature
f we let this
patch aside until upstream has sorted out their stuff?
--
Michael
signature.asc
Description: PGP signature
e, anything I have read on that tells that
it is worth a performance study.
--
Michael
signature.asc
Description: PGP signature
;Administrators" includes "roles with the CREATEROLE privilege" in the
context of GRANTing and REVOKEing default privileges, e.g. with the
attached patch.
Michael
[1] e.g. at https://www.postgresql.org/docs/current/sql-createrole.html
it is mentioned that CREATEROLE privs can be regard
On Fri, Nov 27, 2020 at 11:15:27AM -0500, Stephen Frost wrote:
> * Magnus Hagander (mag...@hagander.net) wrote:
>> On Thu, Nov 26, 2020 at 8:42 AM Michael Paquier wrote:
>>> But here the checksum is broken, so while the offset is something we
>>> can rely on how do yo
be very arduous as a practical
> matter.
Looking at the git history, we have a much larger history of bugs that
relate to race conditions when it comes to LWLocks. I am not really
convinced that it is worth spending time on this even for debug
builds, FWIW.
--
Michael
signature.asc
Description: PGP signature
catalogs by setting
SGML_CATALOG_FILES and XML_CATALOG_FILES. Not sure for SUSE, but one
can use /etc/{sgml,xml}/catalog on Debian, allowing the configure
check to rely on what's stored locally, making configure not rely on
any external resource.
--
Michael
signature.asc
Description: PGP signature
, so may not
> be useful generally. But teaching printf variants about the new format
> would be the best solution. However, I didn't find any way to do that.
-1. %m maps to errno, that is much more generic. A set of macros
that maps to our internal format would be fine enough IMO.
--
Michael
signature.asc
Description: PGP signature
On Wed, Nov 25, 2020 at 04:12:15PM +0900, Michael Paquier wrote:
> Yeah, we had better fix and I guess backpatch something here. That's
> annoying.
I have considered that over the weekend, and let's be conservative.
pg_database has gained a toast table since 12, and before th
ach table
> -referenced by the query. This is no longer allowed.
> -
>
OK to remove the whole paragraph here.
> -
> - The ability to use names to reference SQL function arguments was added
> - in PostgreSQL 9.2. Functions to be used in
> - older s
be less generic than "common", and applied
it. 0002 looks to be in rather good shape, still there are a few
things that have caught my eyes. I'll look at that more closely
tomorrow.
--
Michael
signature.asc
Description: PGP signature
ual new patch. This approach makes for less bloat in
the CF app.
--
Michael
signature.asc
Description: PGP signature
er,
> wouldn't it make sense to roll the resource ownership there as well kind of
> like how JIT is handled? It would make it easier to implement TLS backend
> support, and we won't have to inject OpenSSL headers here.
So, you are referring here about using a new API
/common/. Would you prefer if I send a patch by
myself? Please note that anything I'd send would use directly elog()
and pg_log() instead of returning status codes for the src/common/
routines, and of course not touch ECPG, as that's the approach you are
favoring.
--
Michael
sign
<= 1.8 just with
something like the following:
create extension pageinspect version "1.8";
select get_raw_page('pg_class', 0);
There are no in-core regression tests that check the compatibility of
extensions with older versions, but it is technically possible to do
so.
--
M
t it makes the allocation pattern
easier to follow, similarly to checksum manifests.
Thoughts?
--
Michael
From 3e5c9fa42ff981933bc6eeede92335ae89e94e21 Mon Sep 17 00:00:00 2001
From: Michael Paquier
Date: Tue, 12 Jan 2021 11:21:42 +0900
Subject: [PATCH v5] Refactor hex code
---
src/includ
r at this change, I don't think that this is completely
correct and that could become a trap. This is adding quite a bit of
complexity to take care of contrib_extrasource getting empty, and it
actually overlaps with the handling of OBJS done in AddDir(), no?
--
Michael
signature.asc
Description: PGP signature
atch. I'll go fix it tomorrow if nobody objects.
--
Michael
signature.asc
Description: PGP signature
On Tue, Jan 12, 2021 at 07:15:21PM +1300, Thomas Munro wrote:
> Hah, I even knew that, apparently, but forgot. Adding Michael who
> wrote a patch. It'd be nice to fix this, at least in 14.
Yeah, this rings a bell. I never went back to it even if the thing
looks rather clean at q
On Tue, Jan 12, 2021 at 04:21:18PM +0900, Michael Paquier wrote:
> Good catch. I'll go fix it tomorrow if nobody objects.
And applied. Thanks!
--
Michael
signature.asc
Description: PGP signature
//cfbot.cputube.org/index.html
>
> The key management patch calls the src/common hex functions from
> src/backend/crypto, pg_alterckey, and the crypto tests, and these are
> all tested by make check-world.
Ah, OK. Nice.
--
Michael
signature.asc
Description: PGP signature
; what about something like this? It's just one more para in the "notes"
> section in CREATE INDEX and REINDEX pages, without any additions to the
> VACUUM pages.
+1.
--
Michael
signature.asc
Description: PGP signature
ontext",
> + .phase = RESOURCE_RELEASE_BEFORE_LOCKS,
> + .ReleaseResource = ResOwnerReleaseCryptoHash,
> + .PrintLeakWarning = ResOwnerPrintCryptoHashLeakWarning,
> +};
> +#endif
Looks like a copy-paste error here.
--
Michael
signature.asc
Description: PGP signature
On Tue, Jan 12, 2021 at 11:30:13PM +1300, Thomas Munro wrote:
> I haven't heard any user complaints, and I'd personally be happy with
> a fix on master only.
I have been looking again at that, and the rebased version that Andres
has provided would take care of that. Any thoughts?
ing done by
the latter faster,
--
Michael
signature.asc
Description: PGP signature
eah, that makes sense. I'll send an updated patch based on that.
--
Michael
signature.asc
Description: PGP signature
On Wed, Jan 13, 2021 at 05:22:49PM +0900, Michael Paquier wrote:
> Yeah, that makes sense. I'll send an updated patch based on that.
And here you go as per the attached. I don't think that there was
anything remaining on my radar. This version still needs to be
indented pr
ixed
that, adjusted a couple of elog() strings I found inconsistent after
review and ran pgindent. And applied it.
--
Michael
signature.asc
Description: PGP signature
On Wed, Jan 13, 2021 at 10:11:54PM -0500, Tom Lane wrote:
> Should the CF entry for this be closed? The cfbot is still trying to
> apply the patch, and unsurprisingly failing.
Yes, I was going to close it once I was sure that the buildfarm had
nothing to say. Done now.
--
M
allocates a new string, so the extra
allocation is useless. FWIW, I don't see much point in poking at
the stable branches here.
--
Michael
signature.asc
Description: PGP signature
ched your patch
> (with only this CLUOPT_* correction) to keep it in the thread for sure.
> Although, postgresql.org's web archive is clever enough to link your email
> to the same thread even with different subject.
Oops. Not sure what went wrong here. Thanks.
--
Michael
signature.asc
Description: PGP signature
e//?
> + * CheckPointCompletionTarget used to be exposed as a GUC named
> + * checkpoint_completion_target, but there's little evidence to suggest that
> + * there's actually a case for it being a different value, so it's no longer
> + * exposed as a GUC to be configured.
I would just remove this paragraph.
--
Michael
signature.asc
Description: PGP signature
The current state of things
on HEAD does not sound completely incorrect to me either.
--
Michael
signature.asc
Description: PGP signature
is a patch to remove all those now-useless
declarations, for all four SHA2 options.
Thoughts?
--
Michael
diff --git a/src/include/common/sha2.h b/src/include/common/sha2.h
index f4bae35af1..dfeee6bceb 100644
--- a/src/include/common/sha2.h
+++ b/src/include/common/sha2.h
@@ -18,15 +18,11
or 1.8
and 1.9, but that it's a minor point.
--
Michael
signature.asc
Description: PGP signature
On Wed, Jan 13, 2021 at 04:27:25PM +0900, Michael Paquier wrote:
> I have been looking again at that, and the rebased version that Andres
> has provided would take care of that. Any thoughts?
Hearing nothing, I have applied the thing on HEAD after more tests and
more reads of th
On Fri, Jan 15, 2021 at 03:25:24PM +1300, Thomas Munro wrote:
> Thanks Michael! Another notch for the unnecessary system call
> hitlist: https://wiki.postgresql.org/wiki/Syscall_Reduction
A quick question. How much does it matter in terms of
micro-performance for this code path depending
On Thu, Jan 14, 2021 at 03:11:12PM +0900, Michael Paquier wrote:
> I have just noticed that aef8948 has removed the last reference to
> PG_SHA256_DIGEST_STRING_LENGTH in the code (this was getting used to
> know the length of a SHA256 digest encoded to hex for checksum
> ma
n "Bug fixes". This way, it
won't get lost in the traffic of this list.
--
Michael
signature.asc
Description: PGP signature
and move on. One thing that looks like an
issue to me is that PGOPTIONS is not listed in the section for
environment variables on the docs of pgbench. 5aaa584 has plugged in
a lot of holes, but things could be improved more, for more clients,
where it makes sense of course.
--
Michael
signature.asc
Description: PGP signature
7;
>
> should give you a nearly complete list.
Just to add that all the perl modules we use for the tests are within
src/test/perl/. The coolest tests are within src/bin/ and src/test/.
--
Michael
signature.asc
Description: PGP signature
t introduced tablespaces.
--
Michael
signature.asc
Description: PGP signature
On Thu, Jan 14, 2021 at 02:18:45PM +0900, Michael Paquier wrote:
> Indeed. Let's first wait a couple of days and see if others have any
> comments or objections about this v6.
Hearing nothing, I have looked at that again this morning and applied
v6 after a reindent and some adjustm
pieces remaining. Could it be possible to rebase
things on HEAD and put the tablespace bits into the structures
{Vacuum,Reindex,Cluster}Params?
--
Michael
signature.asc
Description: PGP signature
hat we should map "on" to "md5" rather than "scram-sha-256" on the
side of compatibility. But you have a point when it comes to good
security practices. It makes me wonder whether it would be better to
have pg_dumpall complain rather than pg_upgrade if this value is found
in the proconfig items.. pg_upgrade is not the only upgrade path
supported.
--
Michael
signature.asc
Description: PGP signature
so defined in the NSPR header files, so we need to undef
+ * our version to avoid compiler warnings on redefinition.
+ */
+#define pg_BITS_PER_BYTE BITS_PER_BYTE
+#undef BITS_PER_BYTE
This could be done separately.
src/sgml/libpq.sgml needs to document PQdefaultSSLKeyPassHook_nss, no?
--
Michae
if this input is too late to be helpful.
Cheers,
Michael
external (e.g. either
git.postgresql.org and/or Github), so that it can be picked up by
distributions and/or individual users in need.
That is Assuming it does not need assorted server changes to go with; I
did not read the thread in detail but I was under the assumption it is a
client program?
Mi
function) and some
> function comments which were missing.
0002 (now 0001 I guess) needs a rebase due to a3ed4d1e.
Michael
--
Michael Banck
Projektleiter / Senior Berater
Tel.: +49 2166 9901-171
Fax: +49 2166 9901-100
Email: michael.ba...@credativ.de
credativ GmbH, HRB Mönchengladbach
, and one example of that is
plpgsql_check. We don't have examples on HEAD where
PG_USED_FOR_ASSERTS_ONLY is used on arguments, but that looks to work
properly with gcc.
Thoughts?
--
Michael
diff --git a/src/include/lib/ilist.h b/src/include/lib/ilist.h
index aa196428ed..0c5e1d375f 100644
---
narrow.
check_heap.sql and heap_surgery.sql have one VACUUM FREEZE each and it
seems to me that we had better be sure that no pages are skipped for
their cases?
The duplicated query result looks to be an oversight from 58b4cb3 when
the thing got rewritten, so it can just go away. Good catch.
--
Michael
signature.asc
Description: PGP signature
ng this out entirely, but
> I'd like to know if anybody else does.
Agreed, +1.
--
Michael
signature.asc
Description: PGP signature
* ResourceOwner callbacks to hold JitContexts */
Slight copy-paste error here.
> /*
>* Ensure, while the spinlock's not yet held, that there's a free
> - * refcount entry.
> + * refcount entry and that the resoure owner has room to remember the
> +
x27;d like to add some
coverage as per the attached (the duplicated role name is wanted).
Thoughts?
--
Michael
diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out
index 7754c20db4..873df85e84 100644
--- a/src/test/regress/expected/privileges.out
+++ b/src
CC docs that I looked at is here:
https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#Common-Variable-Attributes
And what I have done does not seem completely legal either for
function arguments, even if I am not getting any complaints when
compiling that.
--
Michael
signature.asc
De
le. Perhaps that's an over-defensive position,
but it does not hurt either to be careful similarly to the test run a
couple of lines above.
--
Michael
signature.asc
Description: PGP signature
t look like
> a missed page would have any effect on the visible results, so I think
> we should leave them alone.
Yeah, I got to wonder a bit about check_btree.sql on a second look,
but that's no big deal to leave it alone either.
> In short I propose the attached patch, which also get
eof(lsnchar), "%X/%X",
> LSN_FORMAT_ARGS(lsn));
>
> This would still reduce notational complexity quite a bit but avoid any
> funny business with the format strings.
That seems reasonable to me. So +1.
--
Michael
signature.asc
Description: PGP signature
espace, the index
+* on its toast table is not normally moved.
*/
Still, REINDEX (TABLESPACE) TABLE should move all of them to be
consistent with ALTER TABLE SET TABLESPACE, but that's not the case
with this code, no? This requires proper test coverage, but there is
nothing o
>> reloid, rd_rel->reltablespace);
>
> Why have a separate "if (changed)" block here instead of merging with
> the above?
Yep.
+ if (SetRelTablespace(reloid, newTableSpace))
+ /* Make sure the reltablespace change
[10,60] range? The gap looks deeper with a low numkeep.
--
Michael
signature.asc
Description: PGP signature
nk that saving nsscontext in pg_cryptohash_ctx is right for each
cryptohash built.
src/tools/msvc/ is missing an update for cryptohash_nss.c.
--
Michael
signature.asc
Description: PGP signature
ing.html
https://www.openssl.org/docs/manmaster/man3/SSL_state_string_long.html
This is interesting for debugging, +1 for applying what you have
here, and this works for 1.0.1~3.0.0. Worth noting that this returns
a static string, as per ssl_stat.c.
--
Michael
signature.asc
Description: PGP signature
501 - 600 of 12108 matches
Mail list logo