Re: Copyright information in source files

2019-11-24 Thread vignesh C
On Sun, Nov 24, 2019 at 7:24 AM John Naylor wrote: > > On Sat, Nov 23, 2019 at 11:39 PM vignesh C wrote: > > > * Copyright (c) 2016-2019, PostgreSQL Global Development Group > > While we're talking about copyrights, I noticed while researching > something else that the PHP project recently got r

Re: dropdb --force

2019-11-24 Thread vignesh C
On Sat, Nov 23, 2019 at 4:42 PM Amit Kapila wrote: > > On Fri, Nov 22, 2019 at 3:16 PM vignesh C wrote: > > > > Thanks for fixing the comments. The changes looks fine to me. I have > > fixed the first comment, attached patch has the changes for the same. > > > > Few comments: > --

Re: dropdb --force

2019-11-24 Thread Pavel Stehule
ne 24. 11. 2019 v 11:25 odesílatel vignesh C napsal: > On Sat, Nov 23, 2019 at 4:42 PM Amit Kapila > wrote: > > > > On Fri, Nov 22, 2019 at 3:16 PM vignesh C wrote: > > > > > > Thanks for fixing the comments. The changes looks fine to me. I have > > > fixed the first comment, attached patch has

[PATCH] Style: fix function declaration

2019-11-24 Thread Ranier Vilela
Hi, Fix function declaration . Best regards, Ranier Vilela --- \dll\postgresql\a\backend\utils\adt\mac8.c 2019-11-23 13:19:20.0 -0300 +++ mac8.c 2019-11-24 09:41:34.200458700 -0300 @@ -35,7 +35,7 @@ #define lobits(addr) \ ((unsigned long)(((addr)->e<<24) | ((addr)->f<<16) | ((a

Re: Copyright information in source files

2019-11-24 Thread Michael Paquier
On Thu, Nov 21, 2019 at 03:42:26PM -0500, Tom Lane wrote: > Yeah, it'd be nice to have some greater consistency there. My own > thought about it is that it's rare to have a file that's *completely* > de novo code, and can be guaranteed to stay that way --- more usually > there is some amount of co

Re: [HACKERS] WAL logging problem in 9.4.3?

2019-11-24 Thread Michael Paquier
On Sat, Nov 23, 2019 at 11:35:09AM -0500, Noah Misch wrote: > That longstanding optimization is too useful to remove, but likely not useful > enough to add today if we didn't already have it. The initial-data-load use > case remains plausible. I can also imagine using wal_level=minimal for data >

[PATCH] Fix var declaration according scanf specification,

2019-11-24 Thread Ranier Vilela
Hi, According to specification of scanf: %x argument must be unsigned. http://www.cplusplus.com/reference/cstdio/scanf/ I think that sscanf must follow scanf specification. Best regards. Ranier Vilela --- \dll\postgresql\a\backend\utils\adt\mac.c 2019-11-23 13:19:20.0 -0300 +++ mac.c

Re: LISTEN/NOTIFY testing woes

2019-11-24 Thread Martijn van Oosterhout
Hoi Tom, On Sun, 24 Nov 2019 at 02:01, Tom Lane wrote: > > This seems both undesirable for our own testing, and rather bogus > from users' standpoints as well. However, I think a simple fix is > available: just make the SQL pg_notification_queue_usage() function > advance the queue tail before m

[PATCH] Possible arithmetic with NULL pointer or test "stack_base_ptr != NULL" is irrelevant.

2019-11-24 Thread Ranier Vilela
Hi, Of course, I don't know if it's the best solution, but it's the most obvious. Or the test at line 3326 is irrelavant. \backend\tcop\postgres.c if (stack_depth > max_stack_depth_bytes && stack_base_ptr != NULL) return true; Otherwise, if is relevant, sub

Re: backup manifests

2019-11-24 Thread David Steele
On 11/23/19 4:34 PM, Andrew Dunstan wrote: > > On 11/23/19 3:13 AM, Tels wrote: >> >> Without the strong hashes it would be pointless to sign the manifest. >> > > I guess I must have missed where we are planning to add a cryptographic > signature. I don't think we were planning to, but the user

Re: Copyright information in source files

2019-11-24 Thread Tom Lane
John Naylor writes: > On Sat, Nov 23, 2019 at 11:39 PM vignesh C wrote: >> * Copyright (c) 2016-2019, PostgreSQL Global Development Group > While we're talking about copyrights, I noticed while researching > something else that the PHP project recently got rid of all the > copyright years from t

Re: LISTEN/NOTIFY testing woes

2019-11-24 Thread Tom Lane
Martijn van Oosterhout writes: > On Sun, 24 Nov 2019 at 02:01, Tom Lane wrote: >> This seems both undesirable for our own testing, and rather bogus >> from users' standpoints as well. However, I think a simple fix is >> available: just make the SQL pg_notification_queue_usage() function >> advan

Re: Copyright information in source files

2019-11-24 Thread Fabien COELHO
Hello Tom, While we're talking about copyrights, I noticed while researching something else that the PHP project recently got rid of all the copyright years from their files, which is one less thing to update and one less cause of noise in the change log for rarely-changed files. Is there ac

[PATCH] Style, remove redudant test "if (zeropadlen > 0)"

2019-11-24 Thread Ranier Vilela
Hi, The test "if (zeropadlen > 0)" is redundant and can be salely removed. It has already been tested in the same path. Best regards, Ranier Vilela --- \dll\postgresql\a\port\snprintf.c 2019-11-23 13:19:20.0 -0300 +++ snprintf.c 2019-11-24 13:02:45.510806400 -0300 @@ -1227,16 +1227,14

Re: [PATCH] Style, remove redudant test "if (zeropadlen > 0)"

2019-11-24 Thread Mark Dilger
On 11/24/19 8:12 AM, Ranier Vilela wrote: Hi, The test "if (zeropadlen > 0)" is redundant and can be salely removed. It has already been tested in the same path. I have not tested your patch, but it looks right to me. -- Mark Dilger

RE: [PATCH] Style, remove redudant test "if (zeropadlen > 0)"

2019-11-24 Thread Ranier Vilela
>I have not tested your patch, but it looks right to me. Thanks for review. Best regards. Ranier Vilela

Re: [PATCH] Style, remove redudant test "if (zeropadlen > 0)"

2019-11-24 Thread John W Higgins
On Sun, Nov 24, 2019 at 8:12 AM Ranier Vilela wrote: > Hi, > The test "if (zeropadlen > 0)" is redundant and can be salely removed. > It has already been tested in the same path. > > Best regards, > Ranier Vilela > > --- \dll\postgresql\a\port\snprintf.c 2019-11-23 13:19:20.0 -0300 > ++

Re: [PATCH] Possible arithmetic with NULL pointer or test "stack_base_ptr != NULL" is irrelevant.

2019-11-24 Thread Tom Lane
Ranier Vilela writes: > Of course, I don't know if it's the best solution, but it's the most obvious. > Or the test at line 3326 is irrelavant. > \backend\tcop\postgres.c > if (stack_depth > max_stack_depth_bytes && > stack_base_ptr != NULL) > return true; > Oth

Re: [PATCH] Style, remove redudant test "if (zeropadlen > 0)"

2019-11-24 Thread Tom Lane
Mark Dilger writes: > On 11/24/19 8:12 AM, Ranier Vilela wrote: >> The test "if (zeropadlen > 0)" is redundant and can be salely removed. >> It has already been tested in the same path. > I have not tested your patch, but it looks right to me. Agreed, seems like an oversight in an old patch of m

RE: [PATCH] Style, remove redudant test "if (zeropadlen > 0)"

2019-11-24 Thread Ranier Vilela
>Could you please at least take the time to produce a patch that actually >applies properly? Yes of course. Thank you. Ranier Vilela

[PATCH] Fix possible underflow in expression (maxoff - 1)

2019-11-24 Thread Ranier Vilela
Hi, The var OffsetNumber maxoff it's like uint16, see at include/storage/off.h typedef uint16 OffsetNumber; Within the function _bt_afternewitemoff, at line 641, maxoff is used in an dangerous expression, without protection.: (maxoff - 1) The function: PageGetMaxOffsetNumber that initializes ma

RE: [PATCH] Possible arithmetic with NULL pointer or test "stack_base_ptr != NULL" is irrelevant.

2019-11-24 Thread Ranier Vilela
>In practice, this code >works fine on every platform that Postgres is ever likely to support, >so I see no need to change it. Of course, I trust your judgment. Thank you for the review. Best regards, Ranier Vilela

Re: LISTEN/NOTIFY testing woes

2019-11-24 Thread Mark Dilger
On 11/23/19 8:50 PM, Mark Dilger wrote: On 11/23/19 5:01 PM, Tom Lane wrote: I ran into a couple of issues while trying to devise a regression test illustrating the LISTEN-in-serializable-transaction issue Mark Dilger reported.  The first one is that an isolation test in which we expect to

Re: LISTEN/NOTIFY testing woes

2019-11-24 Thread Tom Lane
Mark Dilger writes: > On 11/23/19 8:50 PM, Mark Dilger wrote: >> I have finished reading and applying your three patches and have moved >> on to testing them.  I hope to finish the review soon. > After applying all three patches, the stress test that originally > uncovered the assert in predicat

Re: LISTEN/NOTIFY testing woes

2019-11-24 Thread Mark Dilger
On 11/24/19 10:39 AM, Tom Lane wrote: Mark Dilger writes: On 11/23/19 8:50 PM, Mark Dilger wrote: I have finished reading and applying your three patches and have moved on to testing them.  I hope to finish the review soon. After applying all three patches, the stress test that originall

Re: LISTEN/NOTIFY testing woes

2019-11-24 Thread Tom Lane
Mark Dilger writes: > On 11/24/19 10:39 AM, Tom Lane wrote: >> After sleeping on it, I'm not really happy with what I did in >> PrepareTransaction (that is, invent a separate PrePrepare_Notify >> function). The idea was to keep that looking parallel to what >> CommitTransaction does, and preserve

Re: [PATCH] Fix possible underflow in expression (maxoff - 1)

2019-11-24 Thread Peter Geoghegan
On Sun, Nov 24, 2019 at 9:58 AM Ranier Vilela wrote: > Within the function _bt_afternewitemoff, at line 641, maxoff is used in an > dangerous expression, > without protection.: (maxoff - 1) I wrote this code. It's safe. In general, it's not possible to split a page without it being initialized

Re: Collation versioning

2019-11-24 Thread Julien Rouhaud
On Tue, Nov 12, 2019 at 10:16 PM Thomas Munro wrote: > > On Wed, Nov 13, 2019 at 3:27 AM Julien Rouhaud wrote: > > On Sun, Nov 10, 2019 at 10:08 AM Thomas Munro > > wrote: > > > That's because the 0003 patch only calls recordDependencyOnVersion() > > > for simple attribute references. When > >

RE: [PATCH] Fix possible underflow in expression (maxoff - 1)

2019-11-24 Thread Ranier Vilela
>In general, it's not possible to split a page without it being >initialized, and having at least 2 items (not including the incoming >newitem). Besides, even if "maxoff" had an integer underflow the >behavior of the function would still be sane and defined. OffsetNumber >is an unsigned type. Well,

Re: LISTEN/NOTIFY testing woes

2019-11-24 Thread Mark Dilger
On 11/24/19 11:04 AM, Tom Lane wrote: Mark Dilger writes: On 11/24/19 10:39 AM, Tom Lane wrote: After sleeping on it, I'm not really happy with what I did in PrepareTransaction (that is, invent a separate PrePrepare_Notify function). The idea was to keep that looking parallel to what Commi

Re: [PATCH] Fix possible underflow in expression (maxoff - 1)

2019-11-24 Thread Peter Geoghegan
On Sun, Nov 24, 2019 at 11:21 AM Ranier Vilela wrote: > >In general, it's not possible to split a page without it being > >initialized, and having at least 2 items (not including the incoming > >newitem). Besides, even if "maxoff" had an integer underflow the > >behavior of the function would stil

Minor white space typo in documentation

2019-11-24 Thread Andreas Karlsson
Hi, When reading the syntax documentation for SELECT I noticed a missing space in "[ OVERRIDING { SYSTEM | USER} VALUE ]". Andreas diff --git a/doc/src/sgml/ref/insert.sgml b/doc/src/sgml/ref/insert.sgml index f995a7637f1..e829c61642d 100644 --- a/doc/src/sgml/ref/insert.sgml +++ b/doc/src/sg

RE: [PATCH] Fix possible underflow in expression (maxoff - 1)

2019-11-24 Thread Ranier Vilela
>I think that you're being far too optimistic about your ability to >detect and report valid issues using these static analysis tools. It's >not possible to apply the information they provide without a high l>evel understanding of the design of the code. There are already quite >a few full time Pos

Re: [PATCH] Fix possible underflow in expression (maxoff - 1)

2019-11-24 Thread Peter Geoghegan
On Sun, Nov 24, 2019 at 12:02 PM Ranier Vilela wrote: > I've been programming in C for a long time, and I'm getting better every day, > I believe. > I'll arrive there. If you don't understand the *specific* C code in question, you're unlikely to successfully diagnose a problem with the C code. R

Re: Minor white space typo in documentation

2019-11-24 Thread Thomas Munro
On Mon, Nov 25, 2019 at 8:53 AM Andreas Karlsson wrote: > When reading the syntax documentation for SELECT I noticed a missing > space in "[ OVERRIDING { SYSTEM | USER} VALUE ]". Right. Pushed.

Re: Assertion failing in master, predicate.c

2019-11-24 Thread Tom Lane
I wrote: > * Given that the idea is to ignore already-committed entries, it makes > sense that if LISTEN is called inside a serializable transaction block > then the cutoff ought to be the transaction's snapshot. Otherwise we'd > be dropping notifications from transactions that the calling session

Re: TAP tests aren't using the magic words for Windows file access

2019-11-24 Thread Andrew Dunstan
On 11/22/19 3:46 PM, Tom Lane wrote: > Andrew Dunstan writes: >> I think the best course is for us to give your latest patch an outing on >> the buildfarm and verify that the issues seen with slurp_file disappear. >> That shouldn't take us more than a week or two to see - drongo has had 6 >> suc

Re: TAP tests aren't using the magic words for Windows file access

2019-11-24 Thread Tom Lane
Andrew Dunstan writes: > On 11/22/19 3:46 PM, Tom Lane wrote: >> Andrew Dunstan writes: >>> I think the best course is for us to give your latest patch an outing on >>> the buildfarm and verify that the issues seen with slurp_file disappear. >>> That shouldn't take us more than a week or two to s

Re: TAP tests aren't using the magic words for Windows file access

2019-11-24 Thread Andrew Dunstan
On 11/24/19 6:46 PM, Tom Lane wrote: > Andrew Dunstan writes: >> On 11/22/19 3:46 PM, Tom Lane wrote: >>> Andrew Dunstan writes: I think the best course is for us to give your latest patch an outing on the buildfarm and verify that the issues seen with slurp_file disappear. That

Re: [PATCH] Do not use StdRdOptions in Access Methods

2019-11-24 Thread Michael Paquier
On Fri, Nov 22, 2019 at 04:44:50PM +0900, Michael Paquier wrote: > Noted, thanks. I'll sit on this thing for a couple of days, and will > likely look at it again on Monday in order to commit it. And done. -- Michael signature.asc Description: PGP signature

Re: [PATCH] Style: fix function declaration

2019-11-24 Thread Michael Paquier
Hi, On Sun, Nov 24, 2019 at 12:47:40PM +, Ranier Vilela wrote: > Fix function declaration . I see no problem with fixing this kind of inconsistency for readability, so applied the change. Anyway, when sending a patch there are a couple of things which can make the life of people looking at w

RE: [PATCH] Style: fix function declaration

2019-11-24 Thread Ranier Vilela
Hi, >I see no problem with fixing this kind of inconsistency for >readability, so applied the change. Thank you. >Anyway, when sending a patch there are a couple of things which can >make the life of people looking at what you send easier: >https://wiki.postgresql.org/wiki/Submitting_a_Patch

Re: [HACKERS] WAL logging problem in 9.4.3?

2019-11-24 Thread Kyotaro Horiguchi
At Sat, 23 Nov 2019 16:21:36 -0500, Noah Misch wrote in > On Wed, Nov 20, 2019 at 03:05:46PM +0900, Kyotaro Horiguchi wrote: > > By the way, before finalize this, I'd like to share the result of a > > brief benchmarking. > > What non-default settings did you use? Please give the output of this

Re: Safeguards against incorrect fd flags for fsync()

2019-11-24 Thread Michael Paquier
On Thu, Nov 07, 2019 at 01:57:57PM -0800, Mark Dilger wrote: > The code and comments don't clearly indicate what you have said in the > email, that you are verifying directories are opened read-only and files are > opened either read-write or write-only. I'd recommend changing the comments > a bit

Re: Remove configure --disable-float4-byval and --disable-float8-byval

2019-11-24 Thread Michael Paquier
On Thu, Nov 21, 2019 at 07:20:28PM +0100, Peter Eisentraut wrote: > I have committed this part. > > I will rebase and continue developing the rest of the patches based on the > discussion so far. Based on that I am marking the patch as committed in the CF. The rest of the patch set could have a

Re: row filtering for logical replication

2019-11-24 Thread Amit Langote
Hi Euler, Thanks for working on this. I have reviewed the patches, as I too am working on a patch related to logical replication [1]. On Thu, Sep 26, 2019 at 8:20 AM Euler Taveira wrote: > > Em qua, 25 de set de 2019 às 08:08, Euler Taveira > escreveu: > > > > I'll send a patchset later today.

Re: row filtering for logical replication

2019-11-24 Thread Amit Langote
On Mon, Nov 25, 2019 at 11:38 AM Amit Langote wrote: > Needed to be rebased, which I did, to be able to test them; patches attached. Oops, really attached this time. Thanks, Amit 0002-Store-number-of-tuples-in-WalRcvExecResult.patch Description: Binary data 0003-Refactor-function-create_esta

Re: Safeguards against incorrect fd flags for fsync()

2019-11-24 Thread Mark Dilger
On 11/24/19 6:28 PM, Michael Paquier wrote: On Thu, Nov 07, 2019 at 01:57:57PM -0800, Mark Dilger wrote: The code and comments don't clearly indicate what you have said in the email, that you are verifying directories are opened read-only and files are opened either read-write or write-only.

Re: [HACKERS] WAL logging problem in 9.4.3?

2019-11-24 Thread Noah Misch
On Mon, Nov 25, 2019 at 11:08:54AM +0900, Kyotaro Horiguchi wrote: > At Sat, 23 Nov 2019 16:21:36 -0500, Noah Misch wrote in > > This benchmark procedure may help: > > > > 1. Determine $DDL_COUNT, a number of DDL transactions that take about one > >minute when done via syncs. > > 2. Start "p

Re: Attempt to consolidate reading of XLOG page

2019-11-24 Thread Michael Paquier
On Fri, Nov 22, 2019 at 07:56:32PM -0300, Alvaro Herrera wrote: > I see no reason to leave ws_off. We can move that to XLogReaderState; I > did that here. We also need the offset in WALReadError, though, so I > added it there too. Conceptually it seems clearer to me this way. Yeah, that seems c

Fastpath while arranging the changes in LSN order in logical decoding

2019-11-24 Thread Dilip Kumar
In logical decoding, while sending the changes to the output plugin we need to arrange them in the LSN order. But, if there is only one transaction which is a very common case then we can avoid building the binary heap. A small patch is attached for the same. -- Regards, Dilip Kumar EnterpriseD

Re: Safeguards against incorrect fd flags for fsync()

2019-11-24 Thread Mark Dilger
On 11/24/19 6:53 PM, Mark Dilger wrote: On 11/24/19 6:28 PM, Michael Paquier wrote: On Thu, Nov 07, 2019 at 01:57:57PM -0800, Mark Dilger wrote: The code and comments don't clearly indicate what you have said in the email, that you are verifying directories are opened read-only and files

Re: Conflict handling for COPY FROM

2019-11-24 Thread Surafel Temesgen
On Thu, Nov 21, 2019 at 4:22 PM Alexey Kondratov wrote: > > Now the whole patch works exactly as expected for me and I cannot find > any new technical flaws. However, the doc is rather vague, especially > these places: > > + specifying it to -1 returns all error record. > > Actually, we retu

Re: Problem during Windows service start

2019-11-24 Thread Michael Paquier
On Thu, Nov 07, 2019 at 12:55:13PM +0900, Michael Paquier wrote: > So, are there plans to move on with this patch? It is waiting on > author for some time now. Seeing no activity from the author or even the reviewer, I have marked the patch as returned with feedback for now. I am not actually fu

Re: Problem while updating a foreign table pointing to a partitioned table on foreign server

2019-11-24 Thread Michael Paquier
On Tue, Sep 03, 2019 at 12:37:52AM +0900, Etsuro Fujita wrote: > On Wed, Aug 14, 2019 at 11:51 AM Etsuro Fujita > wrote: >> This is my TODO item for PG13, but I'll give priority to other things >> in the next commitfest. If anyone wants to work on it, feel free; >> else I'll move this to the Nov

Re: pg_upgrade fails with non-standard ACL

2019-11-24 Thread Michael Paquier
On Thu, Nov 21, 2019 at 05:53:16PM +0900, Michael Paquier wrote: > Not arguing against the fact that it is useful, but I'd think that it > is a two-step process, where we need to understand what logic needs to > be in the backend or some frontend: > 1) Warn properly about the objects involved, wher

Re: Safeguards against incorrect fd flags for fsync()

2019-11-24 Thread Michael Paquier
On Sun, Nov 24, 2019 at 08:18:38PM -0800, Mark Dilger wrote: > Ok, it passes all regression tests, and I played around with > intentionally breaking the code to open file descriptors in > the wrong mode. The assertion appears to work as intended. > > I'd say this is ready for commit. Thanks for

Re: refactoring - share str2*int64 functions

2019-11-24 Thread Michael Paquier
On Wed, Sep 18, 2019 at 10:13:20AM +0900, Michael Paquier wrote: > As of now, here is an updated patch which takes the path to not > complicate the refactored APIs and fixes the issue with queryID in > readfuncs.c. Thoughts? For now, and seeing that there is little interest in it. I have marked

Re: fix for BUG #3720: wrong results at using ltree

2019-11-24 Thread Michael Paquier
On Thu, Sep 05, 2019 at 04:50:58PM -0400, Alvaro Herrera from 2ndQuadrant wrote: > Hi Oleg, Teodor. Did you find time to refresh your memory on these things? > It would be good to have these bugfixes sorted out. Two months later. Now would be a good time as well! Alexander, you have also looked

Re: [PATCH] Fix Proposal - Deadlock Issue in Single User Mode When IO Failure Occurs

2019-11-24 Thread Michael Paquier
On Mon, Sep 09, 2019 at 05:34:43PM +0530, Amit Kapila wrote: > The only difference is in the last line where for me it gives > assertion failure when trying to do ReleaseAuxProcessResources. Below > is the callstack: No need for Windows on this one and I have reproduced easily the same trace as A

Re: Add A Glossary

2019-11-24 Thread Michael Paquier
On Sat, Nov 09, 2019 at 09:19:16AM +0100, Fabien COELHO wrote: > On principle, I'm fine with having a glossary, i.e. word definitions, which > are expected to be rather stable in the long run. > > I'm wondering whether the effort would not be made redundant by other > on-line effort such as wikipe

Re: Add Change Badges to documentation

2019-11-24 Thread Michael Paquier
On Thu, Nov 07, 2019 at 12:08:43PM +0100, Fabien COELHO wrote: > More precisely, it is a POC to show that the infra works. It adds 3 badges > on various entries. If the final patch could at least finish with one applied, that would be nice as a base example. There are no objections for this patch