Re: psql: \dl+ to list large objects privileges

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.

Re: psql: \dl+ to list large objects privileges

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

Re: Allow batched insert during cross-partition updates

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

Re: [PATCH] postgres-fdw: column option to override foreign types

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

Re: GROUP BY DISTINCT

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

Re: Error on failed COMMIT

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

Re: Display individual query in pg_stat_activity

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

Re: Supporting = operator in gin/gist_trgm_ops

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.

Re: SQL-standard function body

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

Re: Error on failed COMMIT

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

Re: WIP: System Versioned Temporal Table

Hi, just a quick comment that this patch fails on the cfbot. Cheers, //Georgios

Re: Allow an alias to be attached directly to a JOIN ... USING

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

Re: [PATCH] Add extra statistics to explain for Nested Loop

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

Re: Get memory contexts of an arbitrary backend process

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

Re: Display individual query in pg_stat_activity

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

Re: Add session statistics to pg_stat_database

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

Re: Strange behavior with polygon and NaN

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

Re: Error on failed COMMIT

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

Re: pg_upgrade analyze script

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

Re: Extending range type operators to cope with elements

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

Re: Consistent error reporting for encryption/decryption in pgcrypto

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

Re: shared-memory based stats collector

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

Re: New default role- 'pg_read_all_data'

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

Re: v13: show extended stats target in \d

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

Re: New default role- 'pg_read_all_data'

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

Re: New default role- 'pg_read_all_data'

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

Re: [PATCH] Finally split StdRdOptions into HeapOptions and ToastOptions

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,

Re: Using Valgrind to detect faulty buffer accesses (no pin or buffer content lock held)

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

Re: Refactor compile-time assertion checks for C/C++

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

Re: ALTER TEXT SEARCH DICTIONARY tab completion

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

Re: Refactor compile-time assertion checks for C/C++

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

Re: Duplicate Workers entries in some EXPLAIN plans

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

Re: Duplicate Workers entries in some EXPLAIN plans

>> + 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. > > >

Re: Duplicate Workers entries in some EXPLAIN plans

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 >

Re: Duplicate Workers entries in some EXPLAIN plans

> 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

Re: Duplicate Workers entries in some EXPLAIN plans

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

Re: Duplicate Workers entries in some EXPLAIN plans

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

Re: CPU costs of random_zipfian in pgbench

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

Re: CPU costs of random_zipfian in pgbench

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

Re: Adding a TAP test checking data consistency on standby with minRecoveryPoint

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

Re: Adding a TAP test checking data consistency on standby with minRecoveryPoint

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 .

Re: Tighten error control for OpenTransientFile/CloseTransientFile

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

Re: Tighten error control for OpenTransientFile/CloseTransientFile

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