The following review has been posted through the commitfest application:
make installcheck-world: not tested
Implements feature: not tested
Spec compliant: not tested
Documentation:not tested
Hi,
There is something I forgot to mention in my previous review.
Typically
The following review has been posted through the commitfest application:
make installcheck-world: not tested
Implements feature: not tested
Spec compliant: not tested
Documentation:not tested
Hi,
thank you for the patch, I personally think it is a useful addition and
Hi,
thanks for the patch. I had a first look and played around with the code.
The code seems clean, complete, and does what it says on the tin. I will
need a bit more time to acclimatise with all the use cases for a more
thorough review.
I small question though is why expose PartitionTupleRoutin
The following review has been posted through the commitfest application:
make installcheck-world: not tested
Implements feature: not tested
Spec compliant: not tested
Documentation:not tested
Hi,
Thanks for the patch.
I am afraid I will have to :-1: this patch. Of co
The following review has been posted through the commitfest application:
make installcheck-world: not tested
Implements feature: not tested
Spec compliant: not tested
Documentation:not tested
Hi,
this is a useful feature, thank you for implementing. I gather that it f
Hi,
this patch fails on the cfbot yet it has received an update during the current
CF.
I will move it to the next CF and mark it there as Waiting on Author.
Cheers,
Georgios
The new status of this patch is: Needs review
This patch fails in the cfbot for quite some time now.
I shall close it as Return With Feedback and not move it to the next CF.
Please feel free to register an updated version afresh for the next CF.
Cheers,
//Georgios
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: not tested
Documentation:not tested
Hi,
this patch implements a useful and missing feature. Thank you.
Hi,
I noticed that this patch fails on the cfbot.
For this, I changed the status to: 'Waiting on Author'.
Cheers,
//Georgios
The new status of this patch is: Waiting on Author
Hi,
I noticed that this patch fails on the cfbot.
For this, I changed the status to: 'Waiting on Author'.
Cheers,
//Georgios
The new status of this patch is: Waiting on Author
Hi,
just a quick comment that this patch fails on the cfbot.
Cheers,
//Georgios
Hi,
I noticed that this patch fails on the cfbot.
For this, I changed the status to: 'Waiting on Author'.
Cheers,
//Georgios
The new status of this patch is: Waiting on Author
Hi,
I noticed that this patch fails on the cfbot.
For this, I changed the status to: 'Waiting on Author'.
Cheers,
//Georgios
The new status of this patch is: Waiting on Author
Hi,
I noticed that this patch fails on the cfbot.
For this, I changed the status to: 'Waiting on Author'.
Cheers,
//Georgios
The new status of this patch is: Waiting on Author
Hi,
I noticed that this patch is failing on the cfbot.
For this, I changed the status to: 'Waiting on Author'
Cheers,
Georgios
The new status of this patch is: Waiting on Author
Hi,
I noticed that the cfbot fails for this patch.
For this, I am setting the status to: 'Waiting on Author'.
Cheers,
//Georgios
The new status of this patch is: Waiting on Author
Hi,
apologies for the very, very late reply to your fixes.
You have answered/addressed all my questions concerns. The added documentation
reads well, at least to a non native English speaker.
The patch still applies and as far as I can see the tests are passing.
It gets my :+1: and I am changin
Hi,
thank you for your contribution.
I did notice that the cfbot [1] is failing for this patch.
Please try to address the issue for the upcoming commitfest.
Cheers,
//Georgios
[1] http://cfbot.cputube.org/dave-cramer.html
Hi,
thank you for you contribution.
I did notice that the cfbot [1] is failing for this patch.
Please try to address the issues for the upcoming Commitfest.
Cheers,
//Georgios
[1] http://cfbot.cputube.org/magnus-hagander.html
Hi,
thank you for your contribution.
I did notice that the cfbot [1] is failing for this patch.
Please try to address the issues if you can for the upcoming commitfest.
Cheers,
//Georgios
[1] http://cfbot.cputube.org/esteban-zimanyi.html
Hi,
thank you for your contribution.
I did notice that the cfbot [1] is not failing for this patch.
Cheers,
//Georgios
[1] http://cfbot.cputube.org/daniel-gustafsson.html
Hi,
I noticed that according to the cfbot this patch no longer applies.
As it is registered in the upcoming commitfest, it would be appreciated
if you could rebase it.
Cheers,
//Georgios
Hi,
this patch is in "Ready for committer" state and the Cfbot is happy.
Is there any committer that is available for taking a look at it?
Cheers,
//Georgios - CFM 2020-11
The following review has been posted through the commitfest application:
make installcheck-world: not tested
Implements feature: not tested
Spec compliant: not tested
Documentation:not tested
Hi,
I will humbly disagree with the current review. I shall refrain from cha
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: not tested
Spec compliant: not tested
Documentation:not tested
Version 2 of the patch, implements a useful feature. Based on the mailing
Thank you for the patch.
My high level review comment:
The patch seems to be implementing a useful and requested feature.
The patch applies cleanly and passes the basic regress tests. Also the
commitfest bot is happy.
A first pass at the code, has not revealed any worthwhile comments.
Please all
The following review has been posted through the commitfest application:
make installcheck-world: not tested
Implements feature: not tested
Spec compliant: not tested
Documentation:not tested
Hi,
thank you for the patch. It applies cleanly, compiles and passes check,
As a general overview, the series of patches in the mail thread do match their
description. The addition of the stricter, explicit use of instrumentation does
improve the design as the distinction of the use cases requiring a pin or a
lock is made more clear. The added commentary is descriptive
Thank you for updating the status of the issue.
I have to admit that I completely missed the CF bot. Lesson learned.
For whatever is worth, my previous comment that the patch improves
readability also applies to the updated version of the patch.
The CF bot seems happy now, which means that your
The following review has been posted through the commitfest application:
make installcheck-world: not tested
Implements feature: not tested
Spec compliant: not tested
Documentation:not tested
It looks good and does what it says on the tin.
One minor nitpick I feel I s
The following review has been posted through the commitfest application:
make installcheck-world: not tested
Implements feature: not tested
Spec compliant: not tested
Documentation:not tested
In my humble opinion the patch improves readability, hence gets my +1.
No fu
The following review has been posted through the commitfest application:
make installcheck-world: not tested
Implements feature: not tested
Spec compliant: not tested
Documentation:not tested
> Ah, I see. I think I got that from ExplainPrintSettings. I've
> corrected m
>> + int num_workers = planstate->worker_instrument->num_workers;
>> + int n;
>> + worker_strs = (StringInfo *) palloc0(num_workers *
>> sizeof(StringInfo));
>> + for (n = 0; n < num_workers; n++) {
>>
>> I think C99 would be better here. Also no parenthesis needed.
>
>
> P
The following review has been posted through the commitfest application:
make installcheck-world: not tested
Implements feature: not tested
Spec compliant: not tested
Documentation:not tested
> TEXT format was tricky due to its inconsistencies, but I think I have
> som
> Sounds good, I'll try that format. Any idea how to test YAML? With the
> JSON format, I was able to rely on Postgres' own JSON-manipulating
> functions to strip or canonicalize fields that can vary across
> executions--I can't really do that with YAML.
Yes, this approach was clear in the patch
The following review has been posted through the commitfest application:
make installcheck-world: tested, failed
Implements feature: not tested
Spec compliant: not tested
Documentation:not tested
The current version of the patch (v2) applies cleanly and does what it sa
The following review has been posted through the commitfest application:
make installcheck-world: not tested
Implements feature: not tested
Spec compliant: not tested
Documentation:not tested
This is a high level review only. However, seeing that there is a conflict an
The following review has been posted through the commitfest application:
make installcheck-world: not tested
Implements feature: not tested
Spec compliant: not tested
Documentation:not tested
Version 3 of the patch looks ready for committer.
Thank you for taking the t
The following review has been posted through the commitfest application:
make installcheck-world: not tested
Implements feature: not tested
Spec compliant: not tested
Documentation:not tested
For whatever it is worth, the patch looks good to me.
A minor nitpick would
Hi,
you are right in saying that my comment didn't offer much of a constructive
explanation.
Apologies for that.
To the issue at hand. Tests were run in the same manner as in all other cases
and the test
in question was the only one to fail in the whole tree.
By looking a bit closer to the err
Hi,
I applied the patch on current master and run the tests, but I am afraid that
the newly introduced test failed on installcheck-world:
```t/016_min_consistency.pl . # Looks like your test exited with 29
before it could output anything.
t/016_min_consistency.pl . Dubio
The following review has been posted through the commitfest application:
make installcheck-world: tested, failed
Implements feature: not tested
Spec compliant: not tested
Documentation:not tested
The second version of this patch seems to be in order and ready for commi
Overall the patch looks good and according to the previous discussion fulfils
its purpose.
It might be worthwhile to also check for errors on close in SaveSlotToPath().
pgstat_report_wait_end();
CloseTransientFile(fd);
/* rename to permanent file, fsync file and directo
43 matches
Mail list logo