On 06.01.2022 21:13, Tom Lane wrote:
I made the same changes to two files in the 'expected' directory:
largeobject.out and largeobject_1.out.
Although I must say that I still can't understand why more than one file
with expected result is used for some tests.
Because the output sometimes varies
Pavel Luzanov writes:
>> I wonder if we shouldn't put these new tests in largeobject.sql instead.
>> (That would mean there are two expected-files to change, which is a bit
>> tedious, but it's not very hard.)
> I made the same changes to two files in the 'expected' directory:
> largeobject.out
Justin, Tom,
Thanks for the review and the help in splitting the patch into two parts.
I wonder if we shouldn't put these new tests in largeobject.sql instead.
(That would mean there are two expected-files to change, which is a bit
tedious, but it's not very hard.)
As suggested, I moved the
Justin Pryzby writes:
> I suggest to move the function in a separate 0001 commit, which makes no code
> changes other than moving from one file to another.
> A committer would probably push them as a single patch, but this makes it
> easier to read and review the changes in 0002.
Yeah, I agree
On Tue, Aug 31, 2021 at 05:14:12PM +0300, Pavel Luzanov wrote:
> I decided to move the do_lo_list function to describe.c in order to use the
> printACLColumn helper function. And at the same time I renamed do_lo_list to
> listLargeObjects to unify with the names of other similar functions.
The
Hi,
Thank you for testing.
As far as I understand, for the patch to move forward, someone has to become a
reviewer
and change the status in the commitfest app.
Pavel Luzanov
Postgres Professional: https://postgrespro.com
The Russian Postgres Company
On 18.09.2021 05:41, Neil Chen wrote:
The
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: tested, passed
Documentation:tested, passed
Hi, I think this is an interesting patch. +1
I tested it for
Hi,
On 06.09.2021 14:39, gkokola...@pm.me wrote:
Thanks! The tests look good. A minor nitpick would be to also add a comment on
the
large object to verify that it is picked up correctly.
Also:
+\lo_unlink 42
+DROP ROLE lo_test;
+
That last empty line can be removed.
The new
Hi,
On 06.09.2021 14:39, gkokola...@pm.me wrote:
I apply patches by `git apply` and executing the command on the latest version
of your patch, produces:
$ git apply lo-list-acl-v2.patch
lo-list-acl-v2.patch:349: new blank line at EOF.
+
warning: 1 line adds whitespace errors
‐‐‐ Original Message ‐‐‐
On Sunday, September 5th, 2021 at 21:47, Pavel Luzanov
wrote:
Hi,
> Hi,
>
> On 03.09.2021 15:25, Georgios Kokolatos wrote:
>
> > On a high level I will recommend the addition of tests. There are similar
> > tests
>
> Tests added.
Thanks! The tests look good.
Hi,
On 03.09.2021 15:25, Georgios Kokolatos wrote:
On a high level I will recommend the addition of tests. There are similar tests
Tests added.
Applying the patch, generates several whitespace warnings. It will be helpful
if those warnings are removed.
I know this is a silly mistake, and
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.
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
Hello,
Thank you very mush for review.
I will prepare a new version of the patch according to your comments.
For now, I will answer this question:
I will also inquire as to the need for renaming the function `do_lo_list` to
`listLargeObjects` and its move to describe.c. from large_obj.c. In
Hello,
Thank you very much for review.
Since '\dl' already contains description, it might be worthwhile to consider to
add the column `Access privileges`
without introducing a verbose version.
I thought about it.
The reason why I decided to add the verbose version is because of
backward
On 31.08.2021 17:35, Daniel Gustafsson wrote:
Please do, if it was interesting enough for you to write it, it’s
interesting enough to be in the commitfest.
Thanks, added to the commitfest.
Pavel Luzanov
Postgres Professional: https://postgrespro.com
The Russian Postgres Company
> On 31 Aug 2021, at 16:14, Pavel Luzanov wrote:
> If it's interesting, I can add the patch to commitfest.
Please do, if it was interesting enough for you to write it, it’s interesting
enough to be in the commitfest.
--
Daniel Gustafsson https://vmware.com/
Hello,
While working through the documentation I found an empty cell in the
table for the large objects privilege display with the psql command [1].
And indeed the \dl command does not show privileges. And there is no
modifier + for it.
This patch adds a + modifier to the \dl command and
18 matches
Mail list logo
