On Fri, Sep 20, 2019 at 08:58:27PM +0900, Michael Paquier wrote:
> I still need to do an extra pass on the code (particularly the AM
> part), but I think that we could commit that. Please note that I
> included the fix for the lockmode I sent today so as the patch can be
> tes
not been addressed, no? Are we
sure that we want this method if it proves to be inefficient when
there are many sub-contexts and shouldn't we at least test such
scenarios with a worst-case, customly-made, function?
--
Michael
signature.asc
Description: PGP signature
e.
> ...
> case PG_TLS1_1_VERSION:
> #ifdef TLS1_1_VERSION
> return TLS1_1_VERSION;
> #else
> break;
> #endif
> ...
> (line 1290). In this case check for TLS1_1_VERSION <= TLS_MAX_VERSION
> seems to be more self
in the future, of course, once we have
> more practical experience.
>
> Barring objections, I do plan to get this committed by the end of this
> CF (i.e. sometime later this week).
Sounds good to me. Though I have not looked at the patch in details,
the arguments are sensible. Thanks for
On Tue, Sep 24, 2019 at 11:33:35AM +0900, Michael Paquier wrote:
> Thanks for the review. Attached is the patch set I am planning to
> commit. I'll wait after the tag of this week as the first patch needs
> to go down to 9.6, the origin of the bug being 47167b7. The second
> pa
am
discarding that part for now. And we can easily improve it
incrementally.
(One extra thing which is also annoying with the current interface is
that we don't actually pass down the option name within the validator
function for string options like GUCs, so you cannot know on which
option
g
cross-checks about what patterns are allowed or not is a good idea for
all reloption types. I have added all that, and committed the
module. The amount of noise generated by the string validator routine
was a bit annoying, so I have silenced them where they don't really
matter (bas
ails, but I think that
we should not rely on the assumption that no code paths in this area do
not check after CHECK_FOR_INTERRUPTS() as smgrtruncate() does much
more than just the physical segment truncation.
--
Michael
signature.asc
Description: PGP signature
On Tue, Sep 24, 2019 at 11:36:40AM +0900, Amit Langote wrote:
> Sure, thank you.
And done with f5daf7f, back-patched down to 12.
--
Michael
signature.asc
Description: PGP signature
I am not sending a new patch
set just for that change though, and the most recent version is here:
https://github.com/michaelpq/postgres/tree/objaddr_nulls
--
Michael
signature.asc
Description: PGP signature
0.9.8 in HEAD, I won't stand in the way.
Agreed. There is an argument for dropping support for OpenSSL 0.9.8
in 13~, but I don't agree of doing that in 12. Let's just fix the
issue.
--
Michael
signature.asc
Description: PGP signature
lity for potential reviews.
Another key thing to remember is that one patch authored requires one
other patch of equal difficulty to be reviewed.
--
Michael
signature.asc
Description: PGP signature
ment and
replaced by new, equivalent fields, still all those three fields are
still initialized for the palloc_extended() call to allocate
XLogReaderState, while the two others are now part of
WALOpenSegmentInit(). Your change is correct, so applied.
--
Michael
signature.asc
Description: PGP signature
ast 1.0.2 anyway :)
So I agree with the proposal to rely on the presence of
TLS_MAX_VERSION, and base our decision-making on that.
--
Michael
signature.asc
Description: PGP signature
I would also add more brackets around the extra conditions for
readability.
--
Michael
signature.asc
Description: PGP signature
On Wed, Sep 25, 2019 at 09:21:03AM -0300, Alvaro Herrera wrote:
> On 2019-Sep-24, Michael Paquier wrote:
>> + * - FORMAT_TYPE_FORCE_NULL
>> + *if the type OID is invalid or unknown, return NULL instead of ???
>> + *or such
>
> I think FORCE_NULL is a strange name
ose flags consistently
in a stable branch..
--
Michael
signature.asc
Description: PGP signature
te
> September) close this as Returned with Feedback?
That sounds rather right to me.
--
Michael
signature.asc
Description: PGP signature
good too.
Here you go. There were four conflicts in total caused by the switch
to the central logging infra for vacuumlo/oid2name and the
introduction of recovery_gen.c. No actual changes except the rebase.
--
Michael
diff --git a/contrib/oid2name/Makefile b/contrib/oid2name/Makefile
index
On Thu, Sep 26, 2019 at 04:43:33PM +0200, Peter Eisentraut wrote:
> On 2019-09-26 06:53, Michael Paquier wrote:
> > So I agree with the proposal to rely on the presence of
> > TLS_MAX_VERSION, and base our decision-making on that.
>
> But then there
SSL tests on HEAD with at least 0.9.8 as we forgot to add a
conditional check for HAVE_X509_GET_SIGNATURE_NID as c3d41cc did.
I'll send a patch for that separately. That's why I have checked the
patch only with REL_12_STABLE.
--
Michael
signature.asc
Description: PGP signature
hat with OpenSSL 1.0.1 and 1.0.2 to stress
both scenarios.
Any objections to this fix?
Thanks,
--
Michael
diff --git a/src/test/ssl/t/002_scram.pl b/src/test/ssl/t/002_scram.pl
index 5fa2dbde1c..c08aa19aee 100644
--- a/src/test/ssl/t/002_scram.pl
+++ b/src/test/ssl/t/002_scram.pl
@@ -18,11 +18,15 @@
version
number in the comment, which should have been 0.9.6 from the start.
Anyway, let's clean up this code as per the attached. This set of
flags indeed exists since 0.9.7. Any thoughts or objections?
--
Michael
diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-s
xed and committed this way.
(The list of options displayed would be alphabetically ordered for the
completion but it is good to keep the code consistent with the docs,
this makes easier future checks when adding new options).
--
Michael
signature.asc
Description: PGP signature
tstrap mode" | wc -l
68
$ git grep -i "bootstrap process" | wc -l
9
"bootstrapper" sounds weird. My 2c.
--
Michael
signature.asc
Description: PGP signature
a different start
code path.
> The patch refers to a 2017 copyright, that's all I found yet ;o)
Fixed, thanks!
--
Michael
signature.asc
Description: PGP signature
On Thu, Sep 26, 2019 at 01:13:56AM +0900, Fujii Masao wrote:
> On Tue, Sep 24, 2019 at 10:41 AM Michael Paquier wrote:
>> This also points out that there are other things to worry about than
>> interruptions, as for example DropRelFileNodeLocalBuffers() could lead
>> to an ER
te any password exposure hazards.
For now I am switching the patch as returned with feedback as there is
much more to consider. And it did not attract much attention either.
--
Michael
signature.asc
Description: PGP signature
s version, so patching --check offers a
good compromise. Bruce, are you planning to look more at the patch
posted at [1]?
[1]:
https://www.postgresql.org/message-id/392ca335-068d-7bd3-0ad8-fdf0a45d9...@postgrespro.ru
--
Michael
signature.asc
Description: PGP signature
's actually what happened before cbc55da5.
--
Michael
signature.asc
Description: PGP signature
NULL, making a bit more brittle the whole
structure. We may be able to get useful pieces for it, though it is
not clear what would be the benefits. It may help as well to group
that within the thread dedicated to the rework of the reloption APIs
as a preliminary, refactoring step.
--
Michael
On Wed, Sep 04, 2019 at 03:19:57PM +0900, Ian Barwick wrote:
> Ah, good catch, I will update and resubmit.
Ping. Ian, could you update and resubmit please?
--
Michael
signature.asc
Description: PGP signature
ecovery, something that
cbc55da unfortunately broke.
--
Michael
signature.asc
Description: PGP signature
On Fri, Sep 27, 2019 at 03:50:57PM +0200, Peter Eisentraut wrote:
> On 2019-09-27 03:51, Michael Paquier wrote:
>> Your patch does not issue a ereport(LOG/FATAL) in the event of a
>> failure with SSL_CTX_set_max_proto_version(), which is something done
>> when ssl_protocol_v
es postgres -D $PGDATA
What kind of commands and or compilation options do you use?
--
Michael
signature.asc
Description: PGP signature
y between both threads, perhaps more people
could comment there? Here is the most relevant update:
https://www.postgresql.org/message-id/20190917022913.gb1...@paquier.xyz
--
Michael
signature.asc
Description: PGP signature
On Fri, Sep 27, 2019 at 03:46:09PM +0200, Peter Eisentraut wrote:
> Yes, it seems OK to clean this up in master.
Thanks, applied on HEAD.
--
Michael
signature.asc
Description: PGP signature
On Sat, Sep 28, 2019 at 10:52:18PM +0200, Peter Eisentraut wrote:
> committed with that
Thanks, LGTM.
--
Michael
signature.asc
Description: PGP signature
that's
cheap, and we get the future covered.
--
Michael
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 6c69eb6dd7..1e5d1691ee 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -5461,7 +5461,6 @@ validateReco
le waiting for each other
to finish after their validation phase, see:
https://www.postgresql.org/message-id/20190507030756.gd1...@paquier.xyz
https://www.postgresql.org/message-id/20190507032543.gh1...@paquier.xyz
--
Michael
signature.asc
Description: PGP signature
ackend side. If for a reason or another those two code paths begin
to be taken by a backend with InvalidBackendId, then users of the
session start/end hook will need to think how to handle it if they
didn't from the start, which sounds like a good thing to me.
--
Michael
signature.asc
Description: PGP signature
EAM part similar to the check you are adding? My point
is that we should switch to XLOG_FROM_STREAM only if we are in standby
mode, and that's the only place where the startup process looks at
recoveryWakeupLatch.
--
Michael
signature.asc
Description: PGP signature
On Fri, Sep 27, 2019 at 11:44:57AM +0900, Michael Paquier wrote:
> We need to do something similar to c3d41cc for the test, as per the
> attached. I have tested that with OpenSSL 1.0.1 and 1.0.2 to stress
> both scenarios.
>
> Any objections to this fix?
Committed as a12c7
nf. We have replaced
standby_mode by the standby signal file, so including it if present
is consistent with the past as well, no?
--
Michael
signature.asc
Description: PGP signature
dated version patch including the tests. Please
> review it.
Thanks, your test allows to reproduce the original problem, so that's
nice. I don't have much to say, except some improvements to the
comments of the test as per the attached. What do you think?
--
Michael
diff --git a/src
lly runs two tests, one for the failing exit
> code and one for the error message.
Yes. The committed code still works as I would expect. With OpenSSL
<= 1.0.1, I get 10 tests, and 9 with OpenSSL >= 1.0.2. You can check
the difference from test 5 "SCRAM with SSL and channel
nyway as we
run a transaction in the test, and much better than using directly
GetUserIdAndSecContext() which would not assert for an invalid user
ID. And done with e788bd9.
--
Michael
signature.asc
Description: PGP signature
On Mon, Sep 30, 2019 at 05:50:03PM +0900, Fujii Masao wrote:
> Thanks for the review! OK, attached is the patch which also added
> two assertion checks as you described.
Thanks, looks fine. The indentation looks a bit wrong for the
comments, but that's a nit.
--
Michael
si
On Mon, Sep 30, 2019 at 06:20:31PM -0400, David Steele wrote:
> On 9/30/19 5:26 PM, David Fetter wrote:
>>
>> Thanks for doing all this work!
>
> +1!
Thanks, Alvaro!
--
Michael
signature.asc
Description: PGP signature
On Mon, Sep 30, 2019 at 05:07:08PM +0900, Masahiko Sawada wrote:
> Thank you for updating! The comment in your patch is much better.
Thanks, done and back-patched down to 9.5.
--
Michael
signature.asc
Description: PGP signature
workers */
> SignalSomeChildren(SIGTERM,
> - BACKEND_TYPE_NORMAL | BACKEND_TYPE_AUTOVAC |
> - BACKEND_TYPE_BGWORKER);
> + BACKEND_TYPE_NORMAL | BACKEND_TYPE_WORKER);
Okay for this one.
--
Michael
signature.asc
Description: PGP signature
y break the invariant that we're not in a
> transaction after that point.
Still, it would mean that any module hooking in the area would need to
do the cleanup all the time...
--
Michael
signature.asc
Description: PGP signature
ply to the planner
or post-parse hooks). Should we finally add a specific section in the
user-visible docs even if there has been reluctance to do so? My
guess is that we go down to this kind of requirement if we want to be
able to never forget to add documentation for any kind of new hook.
--
Michael
signature.asc
Description: PGP signature
into 13.
Here are also some guidelines:
https://wiki.postgresql.org/wiki/Submitting_a_Patch
But you are already aware of anyway, right? :p
--
Michael
signature.asc
Description: PGP signature
'pg_rewind local with -R');
Incompatible options had better be checked within a separate perl
script? We generally do that for the other binaries.
--
Michael
diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c
index a7fd9e0cab..2cc32cd404 100644
--
On Wed, Oct 02, 2019 at 09:09:53PM -0700, Andres Freund wrote:
> On 2019-10-03 11:03:39 +0900, Michael Paquier wrote:
>> The idea here is to have a hook which can be triggered at the start of
>> a process which can be externally triggered, which I guess is normal
>> eve
_invalid_pages
> developer parameter. When this parameter is set to true, the startup process
> always ignores invalid-pages errors. Thought?
That could be helpful.
--
Michael
signature.asc
Description: PGP signature
XX000: unexpected relkind: 116
The comment about OBJECT_* in get_relkind_objtype() is here because
there is no need for toast objects to have object address support
(there is a test in object_address.sql about that), and ObjectTypeMap
has no mapping OBJECT_* <-> toast table, so the chan
On Thu, Oct 03, 2019 at 12:43:37PM +0300, Alexey Kondratov wrote:
> On 03.10.2019 6:07, Michael Paquier wrote:
>> I have reworked your first patch as per the attached. What do you
>> think about it? The part with the control file needs to go down to
>> v12, and I would likel
27;s precedent
> for that in OBJECT_ROUTINE ... but I don't know that we want to
> build out all the other infrastructure for a new ObjectType right now.
I am too lazy to check the thread that led to 8b9e964, but I recall
that Peter wanted to get rid of OBJECT_RELATION because that
the common pattern these days, because that's more
performant. I think that the patch should have regression tests.
--
Michael
signature.asc
Description: PGP signature
at the process reading the parameters is not
confused either by default values. It seems to me that if we are not
in recovery, the best thing was can do now is just to not process
anything those parameters trigger, instead of "ignoring" these (you
are referring to use SetConfigOption in the startup process here?).
--
Michael
signature.asc
Description: PGP signature
On Sun, Oct 06, 2019 at 09:06:44AM +0530, vignesh C wrote:
> I could not find the equivalent links for the same.
> Should we update the links for the same?
If it could be possible to find equivalent links which could update
to, it would be nice.
--
Michael
signature.asc
Descriptio
to make sure that the source server is
shut down cleanly before using it. I have included that part as
well, as the flow feels right.
So, Alexey, what do you think?
--
Michael
diff --git a/src/bin/pg_rewind/t/001_basic.pl b/src/bin/pg_rewind/t/001_basic.pl
index c3293e93df..645dc24578 100644
---
o not have any options (you will not find RELOPT_KIND_PARTITIONED in
> boolRelOpts, intRelOpts, realRelOpts, stringRelOpts and enumRelOpts in
> reloption.c), but it uses StdRdOptions to store them (these no options).
I am not even sure that we actually need that. What kind of reloption
you woul
it looks good to me. No spots are visibly missing.
--
Michael
signature.asc
Description: PGP signature
It
seems to me that we should not silently break things.
--
Michael
signature.asc
Description: PGP signature
the attached patch.
Could it be possible to keep the discussion on the original thread? I
already replied to it, and there are no problems with discussing
patches dealing with bugs directly on pgsql-bugs. Thanks for caring.
Thanks,
--
Michael
signature.asc
Description: PGP signature
being done by pg_strtoint32_check.
pg_strtoint32_check is used for a signed integer, so it would complain
about incorrect input syntax, but not when the parsed integer is less
or equal than 0, which is what refint.c complains about.
--
Michael
signature.asc
Description: PGP signature
.
> https://www.postgresql.org/message-id/20191006074122.GC14532%40paquier.xyz
I may be missing something of course, but I do not see a patch neither
on this message nor on the previous one. Could you send a patch you
think is correct?
--
Michael
signature.asc
Description: PGP signature
On Mon, Oct 07, 2019 at 03:31:45PM +0300, Alexey Kondratov wrote:
> On 07.10.2019 4:06, Michael Paquier wrote:
>> - --dry-run coverage is basically the same with the local and remote
>> modes, so it seems like a waste of resource to run it for all the
>> tests and all the mod
n for the patch. I have checked your suggestions and it
looks good to me, so committed. Good idea to tell about
WIN32_NO_STATUS. I have noticed one typo on the way.
--
Michael
signature.asc
Description: PGP signature
On Mon, Oct 07, 2019 at 02:14:05PM +0530, vignesh C wrote:
> Sorry Michael for the miscommunication, the patch was present in the
> first mail of this mail thread.
> I'm re-attaching the patch in this mail.
> Let me know if anything is required.
Thanks. It looks like I have been
doing that. Historically O_RDONLY is 0,
so when looking at a directory we just need to make sure that no write
flags are used. For files, that's the contrary, a write flag has to
be used.
Thoughts or better ideas?
Thanks,
--
Michael
diff --git a/src/backend/storage/file/fd.c b/src/ba
TExecDropColumn
calls itself. And it is not actually as bad as I assumed, please see
the attached.
--
Michael
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 05593f3316..87c39ad6df 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecm
ually
done, and I got some pushback not long ago regarding some undocumented
APIs in libpq that we still ship for the exact same reasons:
https://www.postgresql.org/message-id/7990.1565550...@sss.pgh.pa.us
--
Michael
signature.asc
Description: PGP signature
regarding that aspect, an assertion, and more tests with a partitioned
table without any children, and an actual check that all columns have
been dropped in the leaves of the partition tree. How does that look?
--
Michael
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmd
On Fri, Oct 04, 2019 at 05:55:40PM +0900, Michael Paquier wrote:
> On Thu, Oct 03, 2019 at 09:52:34AM -0400, Tom Lane wrote:
>> FWIW, I really dislike this patch, mainly because it is based on the
>> assumption (as John said) that get_relkind_objtype is used only
>> in
s to be dropped must now be in addrs, so drop. */
I think that it would be better to clarify as well that this stands
after all the child relations have been checked, so what about that?
"The resursive lookup for inherited child relations is done. All the
child relations have been scanned and the object addresses of all the
columns to-be-dropped are registered in addrs, so drop."
--
Michael
signature.asc
Description: PGP signature
renaming scram_build_verifier()
to scram_build_secret() you are going to break one of my in-house
extensions. I am using it to register for a user SCRAM veri^D^D^D^D
secrets with custom iteration and salt length :)
--
Michael
signature.asc
Description: PGP signature
Since the optimizer is choosing a seq scan over index scan when it seems
like it has good row estimates in both cases, to me that may mean costs of
scanning index are expected to be high. Is this workload on SSD? Has the
random_page_cost config been decreased from default 4 (compared with cost
of 1
On Thu, Oct 10, 2019 at 6:22 PM David Rowley
wrote:
> The planner might be able to get a better estimate on the number of
> matching rows if the now() - interval '10 days' expression was
> replaced with 'now'::timestamptz - interval '10 days'. However, care
> would need to be taken to ensure the
; Okay, sure. Maybe it's better to write the comment inside the if
> block, because if recursing is true, we don't drop yet.
Sure.
> Thoughts on suggestion to expand the test case?
No objections to that, so done as per the attached. Does that match
what you were thinking about?
On Fri, Oct 11, 2019 at 04:23:51PM +0900, Amit Langote wrote:
> Thanks. The index on b is not really necessary for testing because it
> remains unaffected, but maybe it's fine.
That's on purpose. Any more comments?
--
Michael
signature.asc
Description: PGP signature
On Fri, Oct 11, 2019 at 06:39:47PM -0300, Alvaro Herrera wrote:
> Typo "resursing". This comment seems a bit too long to me. Maybe
> "Recursion having ended, drop everything that was collected." suffices.
> (Fits in one line.)
Sounds fine to me, thanks.
--
Michae
ce
anything. Perhaps you have some concurrent operations going on?
> Unfortunately, there was no core file, and I'm still trying to reproduce it.
Forgot to set ulimit -c? Having a backtrace would surely help.
--
Michael
signature.asc
Description: PGP signature
:1391
#12 0x564c34ec0d3d in main (argc=3, argv=0x564c35bdefd0) at main.c:210
This means that all the locks hold have not actually been released
when the timeout has kicked in. Not sure that this is only an issue
related to REINDEX CONCURRENTLY, but if that's the case then we are
missing a cleanup step.
--
Michael
signature.asc
Description: PGP signature
we have a way to reproduce
that case rather reliably with an isolation test? The patch looks to
good to me, these are also the two places I spotted yesterday after a
quick lookup. The only other caller is isTempNamespaceInUse() which
does its thing correctly.
--
Michael
signature.asc
Description: PGP signature
ontrolFile(). alterSystemSetConfigFile() writes, but you didn't add
> one.
Because I have not considered these when looking at transient files.
That may be worth an extra lookup.
--
Michael
signature.asc
Description: PGP signature
On Sat, Oct 12, 2019 at 03:08:41PM +0900, Michael Paquier wrote:
> On Fri, Oct 11, 2019 at 06:39:47PM -0300, Alvaro Herrera wrote:
>> Typo "resursing". This comment seems a bit too long to me. Maybe
>> "Recursion having ended, drop everything that was collected.&q
On Mon, Oct 14, 2019 at 08:57:16AM +0900, Michael Paquier wrote:
> I need to think about that, but shouldn't we have a way to reproduce
> that case rather reliably with an isolation test? The patch looks to
> good to me, these are also the two places I spotted yesterday after a
Thanks for closing the loop on the data correlation question. I've been
playing with BRIN indexes on a log table of sorts and this thread helped
clear up some of the behavior I have been seeing.
I am curious, would a partial btree index fit your needs? Perhaps the
maintenance overhead is too signi
On Mon, Oct 14, 2019 at 09:48:12PM +0530, vignesh C wrote:
> About pg_crc.h, I have made the changes with the correct links.
> The patch for the same is attached.
Confirmed, so applied. Thanks, Vignesh.
--
Michael
signature.asc
Description: PGP signature
On Wed, Oct 16, 2019 at 09:53:56AM -0300, Alvaro Herrera wrote:
> Thanks, pushed.
Thanks, Alvaro.
--
Michael
signature.asc
Description: PGP signature
eaplocktag, AccessExclusiveLock, true);
Thoughts?
--
Michael
signature.asc
Description: PGP signature
hanks for noticing that. I am sending an updated
patch set in a but, Alvaro had more comments.
--
Michael
signature.asc
Description: PGP signature
On Thu, Sep 26, 2019 at 03:52:03PM +0900, Michael Paquier wrote:
> On Wed, Sep 25, 2019 at 09:21:03AM -0300, Alvaro Herrera wrote:
>> On 2019-Sep-24, Michael Paquier wrote:
>>> + * - FORMAT_TYPE_FORCE_NULL
>>> + * if the type OID is invalid or unknown, return NULL
Hi all,
I have just bumped into $subject, and we now use the table_*
equivalents in the code. Any objections to the simple patch attached
to clean up that?
Thanks,
--
Michael
diff --git a/src/backend/commands/statscmds.c b/src/backend/commands/statscmds.c
index f51eb7bb64..0ab43deb71 100644
nice to test some scenario within 002_archiving.pl.
--
Michael
signature.asc
Description: PGP signature
ee7c637196a
author: Peter Eisentraut
date: Tue, 26 Feb 2008 06:41:24 +
Refactor the code that creates the shared library export files to appear
only once in Makefile.shlib and not in four copies.
--
Michael
signature.asc
Description: PGP signature
1601 - 1700 of 11810 matches
Mail list logo
