Re: [HACKERS] Small patch for pg_basebackup argument parsing

The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: tested, passed Documentation:not tested I applied this patch via patch -p1. (Had an issue using git

Re: [HACKERS] Small patch for pg_basebackup argument parsing

t; > > On 05 Jul 2017, at 08:32, Michael Paquier <michael.paqu...@gmail.com> > > > wrote:> > > > On Wed, Jul 5, 2017 at 2:57 PM, Ryan Murphy <ryanfmur...@gmail.com> > wrote: > > >> I tried to apply your patch to test it (though reading Robert

Re: [HACKERS] postgres_fdw bug in 9.6

> I tested that with HEAD, but couldn't reproduce this problem. Didn't you test that with HEAD? Yeah, I know it's not an issue other people are having - I think it's something specific about how my environment / postgres build is set up. In any case I knew that it wasn't caused by your patch.

Re: [HACKERS] postgres_fdw bug in 9.6

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 This feature is obviously a pretty deep cut, and I don't understand the

Re: [HACKERS] Useless code in ExecInitModifyTable

> FWIW, I find that good ol' patch is much more reliable about applying > patches that didn't come from git itself. In this case > > Thanks, I knew I was missing something! Ryan

Re: [HACKERS] Useless code in ExecInitModifyTable

Hello! I downloaded the latest patch "clean-nodeModifyTable-v2.patch" and tried applying it to HEAD using "git apply ". The only response from git was "fatal: unrecognized input". I tried this with git 2.5.4 stable that comes native on my mac, and git 2.12 built from source. In both cases I got

Re: [HACKERS] assorted code cleanup

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 I've reviewed the code changes, and it's pretty clear to me

Re: [HACKERS] 1 test fails in make installcheck-world - database "regress_ecpg_user2" does not exist

> Thanks for reporting it! > My pleasure! So the initial issue didn't happen the 2nd time. So if misc_sanity was the only test failing then I guess my tests are working fine other than that. Sweet! When I get a break from work I'll review some patches! Best, Ryan

Re: [HACKERS] 1 test fails in make installcheck-world - database "regress_ecpg_user2" does not exist

> No, you're reading it backwards: the error is expected, but it's not > appearing in your results. I can duplicate this if I manually create > database "regress_ecpg_user2" before running ecpg's installcheck, > so I guess that's what you did. I can find no evidence that any > part of the PG

[HACKERS] 1 test fails in make installcheck-world - database "regress_ecpg_user2" does not exist

Hello Postgres Hackers, I want to become more helpful to the project, reviewing more patches and starting to write more of my own - and one of the first steps is to get all the tests passing so I can confidently review patches. It almost worked... I typed "make check" and all the tests passed.

Re: [HACKERS] Bug in Physical Replication Slots (at least 9.5)?

Poking this. Looking back through the discussion, this issue has been reproduced by multiple people. The patch still applies to HEAD without issues. I have no experience with PostgreSQL replication, so I'm not qualified to really review this. From what I can see with the patch, it's just a

[HACKERS] Re: Error-like LOG when connecting with SSL for password authentication

Hi Michael, I tried to apply your patch to HEAD and it failed. But I think that's because some version of it has already been committed, correct? I see some of your ECONNRESET and "SSL connection has been closed unexpectedly" code already in HEAD. Best, Ryan The new status of this patch

Re: [HACKERS] CommitFest 2017-09 - How do I know what commit to apply patches to

Thanks guys! The expectation is that the patch will be applied to HEAD, so it should > apply to HEAD. If not, the custom is to ask the author for a rebase. > > Makes sense! I'll do that. (I've taken to running a cronjob that tells me when patches I've > posted no longer apply, so I can rebase

Re: [HACKERS] Small patch for pg_basebackup argument parsing

Hi Pierre, I tried to apply your patch to test it (though reading Robert's last comment it seems we wish to have it adjusted before committing)... but in any case I was not able to apply your patch to the tip of the master branch (my git apply failed). I'm setting this to Waiting On Author

[HACKERS] CommitFest 2017-09 - How do I know what commit to apply patches to

Hello PostgreSQL hackers, I was diving into CommitFest 2017-09 to help review some patches, but I was not sure which version / git commit / git tag of the PostgreSQL repo I should be checked out to in order to correctly apply / test these patches. Is there

Re: [HACKERS] Incorrect mentions to pg_xlog in walmethods.c/h

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 commit only affects comments, so I'm confident it doesn't break code,

Re: [HACKERS] Does having a NULL column automatically exclude the table from the tupleDesc cache?

> > You'll probably want to do those at a C level, bypassing the executor. I > > would guess that executor overhead will completely swamp the effect of > the > > cache in most cases. > > That seems like it's kind of missing the point. If the tupleDesc > cache saves so little that it's irrelevant

Re: [HACKERS] Does having a NULL column automatically exclude the table from the tupleDesc cache?

> attcacheoff can only be set positive for fields preceding any varlena > (typlen<0, but including the first such) or nullable values. I don't > know how much faster it is with the cache; you can measure it if your > curiosity is strong enough -- just set the first column to nullable. > > Thanks!

Re: [HACKERS] Does having a NULL column automatically exclude the table from the tupleDesc cache?

> No, that tests whether the particular tuple contains any null fields, not > whether the whole table is declared NOT NULL. > > regards, tom lane > Ok, thanks, that makes sense. My question kind of remains though - does that mean that having any nulls in the tuple loses

[HACKERS] Does having a NULL column automatically exclude the table from the tupleDesc cache?

Hi all, I was looking through some of the implementation details of the heap/tuples, specifically src/include/access/htup_details.h - and I came across the big macro fastgetattr, and had a question about it. I've included the code here for clarity and convenience: #define fastgetattr(tup,

Re: [HACKERS] Access inside pg_node_tree from query?

> > There are no in-core operators or functions to manipulate pg_node_tree. > Thanks Michael, just checking!

[HACKERS] Access inside pg_node_tree from query?

Hi Postgressers, Quick question, just curious - is there a way to access the members of a `pg_node_tree` value within a Postgres query? By pg_node_tree I mean for example, the `ev_qual` field in the `pg_rewrite` table. By "access the members" I mean in the same way that you can access the

Re: [HACKERS] Re: Clarifying "server starting" messaging in pg_ctl start without --wait

> The on-screen output isn't all that helpful for diagnosing what went > wrong. You might learn more by looking at the regression.diffs files. > Remember that errors tend to cascade, so the first one(s) in any > particular test suite are the most important --- the rest might just > be fallout. >

Re: [HACKERS] Re: Clarifying "server starting" messaging in pg_ctl start without --wait

> > Jim Nasby said I shouldn't necessarily need to build the docs / the whole > world in order to review patches. But the Review form needs a `make > installworld-check`. Do I need to install the whole world in order to meet > this requirement? Happy to do so if required, but in that case, I

Re: [HACKERS] Re: Clarifying "server starting" messaging in pg_ctl start without --wait

> > > Ryan try to run 'make install-world' then 'make -i installcheck-world', -i > option will ignore the error and proceed. You can check if any other tests > fails. This is a separate issue, unrelated to this patch. I do not think > we should stop from changing the status because of this. > >

Re: [HACKERS] Re: Clarifying "server starting" messaging in pg_ctl start without --wait

Thanks for the review Beena, I'm glad the patch is ready to go! I think because of my environment/setup, I get errors when I try "make install-world", but I'm at work now, when I have time I will go back and try again and figure out what is wrong. I'll let you guys know if I have any questions.

Re: [HACKERS] Adding type info etc for inheritance errmsg: "child table is missing column ..."

> AFAICS, what Ryan is after would be better served by a separate tool that > would produce a "diff" of the two tables' schemas. Related to the other idea of seeing the problems that exist in all the > columns (instead of one column at a time), I think it'd be reasonable to > have a SRF that spit

Re: [HACKERS] Adding type info etc for inheritance errmsg: "child table is missing column ..."

> Perhaps the ERROR message should remain as is, and a DETAIL or HINT line > could be emitted with the entire column definition (or close to it)? > > I'm open to this option as well. The only parts I really care about are the type and the NOT NULL, since those are the ones that will give you an

Re: [HACKERS] Adding type info etc for inheritance errmsg: "child table is missing column ..."

> No, and TBH I would vote strongly against including that much detail in > this error message anyway. That info could be indefinitely long, and it's > not especially relevant to the stated error condition --- for example, the > presence of a default is *not* relevant to whether the column

Re: [HACKERS] Adding type info etc for inheritance errmsg: "child table is missing column ..."

> Thanks Tom, I'll redo the patch using one of those, and get back to you > guys. > > So I tried using format_type_with_typemod() thinking that the "typemod info" meant things like NOT NULL, DEFAULT etc. It builds and includes the plain type but not all that stuff. E.g. user=# alter table temp

Re: [HACKERS] Adding type info etc for inheritance errmsg: "child table is missing column ..."

> The approved way to do this is with format_type_be(), or > format_type_with_typemod() if you want to include typmod info, which > I think you probably do for the intended use-case. > > regards, tom lane > Thanks Tom, I'll redo the patch using one of those, and get back

[HACKERS] Adding type info etc for inheritance errmsg: "child table is missing column ..."

Hey Postgres team! I've been gleefully using the inheritance feature of Postgres for the last 6 months or so, and it's really great! I especially like how easily you can ALTER TABLE foo INHERIT bar, and get helpful error messages about what columns need to be there before the inherit can take

[HACKERS] Re: Clarifying "server starting" messaging in pg_ctl start without --wait

The following review has been posted through the commitfest application: make installcheck-world: tested, failed Implements feature: tested, passed Spec compliant: tested, passed Documentation:tested, passed (Though I could not check "make installcheck-world" as

Re: [HACKERS] Clarifying "server starting" messaging in pg_ctl start without --wait

> Making --wait the default may or may not be sensible -- I'm not sure > -- but removing --no-wait is clearly a bad idea, and we shouldn't do > it. The fact that the problems created by removing it might be > solvable doesn't mean that it's a good idea to create them in the > first place. > > > I

[HACKERS] Clarifying "server starting" messaging in pg_ctl start without --wait

Hi Postgres Devs, I had a suggestion regarding the output pg_ctl gives when you use it to start the postgres server. At first I was going to write a patch, but then I decided to just ask you guys first to see what you think. I had an issue earlier where I was trying to upgrade my postgres

Re: [HACKERS] Trouble building uuid-ossp extension in new versions of Postgres

> > On Mac, the recommended thing is to forget about the ossp code and >> use "configure --with-uuid=e2fs". Sorry if that wasn't clear enough. >> >> > Ok thanks, I'll try this. > Thanks Tom, "uuid-ossp" built perfectly with "--with--uuid=e2fs". Cheers and Happy Holidays! Ryan

Re: [HACKERS] Trouble building uuid-ossp extension in new versions of Postgres

> On Mac, the recommended thing is to forget about the ossp code and > use "configure --with-uuid=e2fs". Sorry if that wasn't clear enough. > > Ok thanks, I'll try this. > Reading over your post again, it sounds like you're trying to force-build > contrib/uuid-ossp without having used any of

[HACKERS] Trouble building uuid-ossp extension in new versions of Postgres

Hello, I have been trying to build Postgres and migrate my data to the newest version. Postgres builds just fine, but I also need the uuid-ossp module, which used to build fine for me and now does not... I am currently "git pull"ed to commit b645a05fc6112a4857ceac574d4aa2 4174a70417. I cd into

Re: [HACKERS] proposal: psql \setfileref

> > please, can you check attached patch? It worked in my laptop. > > Regards > > Pavel > > Yes, that one applied for me without any problems. Thanks, Ryan

Re: [HACKERS] proposal: psql \setfileref

Hi Pavel, I just tried to apply your patch psql-setfileref-initial.patch (using git apply) to the newest revision of postgres at the time (da6c4f6ca88) and it failed to patch startup.c. Thinking that the patch was for some previous revision, I then checked out d062245b5, which was from

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

Thanks for committing it Tom! Thank you all for your help. I really like the Postgres project! If there's anything that especially needs worked on let me know, I'd love to help. Best, Ryan

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

> > I looked into this and soon found that fe_utils/string_utils.o has > dependencies on libpq that are much wider than just pqexpbuffer :-(. > It might be a project to think about sorting that out sometime, but it > looks like it would be an awful lot of work just to avoid having initdb > depend

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

On Sat, Aug 20, 2016 at 8:26 AM, Michael Paquier <michael.paqu...@gmail.com> wrote: > On Sat, Aug 20, 2016 at 4:39 AM, Ryan Murphy <ryanfmur...@gmail.com> > wrote: > > Here is another version of my initdb shell quoting patch. I have removed > > the unnecessary

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

Here is another version of my initdb shell quoting patch. I have removed the unnecessary {} block. I also ran pgindent on the code prior to creating the patch. On Thu, Aug 18, 2016 at 3:50 PM, Ryan Murphy <ryanfmur...@gmail.com> wrote: > > > On Thu, Aug 18, 2016 at 3:44

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

On Thu, Aug 18, 2016 at 3:44 PM, Tom Lane <t...@sss.pgh.pa.us> wrote: > Ryan Murphy <ryanfmur...@gmail.com> writes: > > On Thu, Aug 18, 2016 at 3:22 PM, Robert Haas <robertmh...@gmail.com> > wrote: > >>>> +{ /* pg_ctl command w path, prope

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

On Thu, Aug 18, 2016 at 3:22 PM, Robert Haas wrote: > On Thu, Aug 18, 2016 at 4:15 PM, Andres Freund wrote: > > On 2016-08-18 16:11:27 -0400, Robert Haas wrote: > >> On Wed, Aug 17, 2016 at 11:18 PM, Andres Freund > wrote: > >> >

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

aquier wrote: > > On Thu, Aug 18, 2016 at 12:21 AM, Ryan Murphy <ryanfmur...@gmail.com> > wrote: > > > I have created a better patch (attached) that correctly escapes the > shell > > > arguments using PQExpBufferStr and the appendShellString function, as > per > &g

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

Thu, Aug 18, 2016 at 12:21 AM, Ryan Murphy <ryanfmur...@gmail.com > <javascript:;>> wrote: > > I have created a better patch (attached) that correctly escapes the shell > > arguments using PQExpBufferStr and the appendShellString function, as per > > Michae

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

I have created a better patch (attached) that correctly escapes the shell arguments using PQExpBufferStr and the appendShellString function, as per Michael and Andres' suggestions. Further suggestions welcome of course. Ryan On Wed, Aug 17, 2016 at 8:28 AM, Ryan Murphy <ryanfmur...@gmail.

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

That makes sense, Michael and Andres. I started to make a solution that uses a PQExpBuffer, appendShellString, etc. I think it will work just fine, but I think I need to alter the Makefile as well, to get initdb.c to be compiled using -L../../../src/fe_utils -lpgfeutils. Otherwise I am having

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

I've submitted my patch to Commitfest 2016-09. https://commitfest.postgresql.org/10/718/ My username on postgresql.org is murftown On Tue, Aug 16, 2016 at 1:02 AM, Ryan Murphy <ryanfmur...@gmail.com> wrote: > Ok, I'll do that! > Thanks Michael! > Ryan > > > On Monday,

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

Ok, I'll do that! Thanks Michael! Ryan On Monday, August 15, 2016, Michael Paquier <michael.paqu...@gmail.com> wrote: > On Mon, Aug 15, 2016 at 12:12 AM, Ryan Murphy <ryanfmur...@gmail.com > <javascript:;>> wrote: > > This is to fix an issue that came

[HACKERS] Small patch: initdb: "'" for QUOTE_PATH (non-windows)

Hello Postgres Hackers! I sent this already a few hours ago but it got blocked because I had not yet joined the mailing list. Trying again, sorry for any redundancy or confusion! This is to fix an issue that came up for me when running initdb. At the end of a successful initdb it says:

[HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

Hello Postgres Team! This is to fix an issue that came up for me when running initdb. At the end of a successful initdb it says: Success. You can now start the database server using: pg_ctl -D /some/path/to/data -l logfile start but this command did not work for me because my data