Re: [HACKERS] Patch: add --if-exists to pg_recvlogical

2017-11-01 Thread Catalin Iacob
On Fri, Sep 29, 2017 at 3:06 PM, Peter Eisentraut wrote: > On 9/22/17 15:31, Peter Eisentraut wrote: >> I suggest you create a patch that focuses on the --create part. >> >> You can create a separate follow-on patch for the --start part if you >> wish, but I

Re: [HACKERS] [bug fix] Savepoint-related statements terminates connection

2017-09-08 Thread Catalin Iacob
On Thu, Sep 7, 2017 at 8:07 PM, Tom Lane wrote: > I've pushed up an attempt at this: > > https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=b976499480bdbab6d69a11e47991febe53865adc > > Feel free to suggest improvements. Thank you, this helps a lot. Especially

Re: [HACKERS] [bug fix] Savepoint-related statements terminates connection

2017-09-05 Thread Catalin Iacob
On Mon, Sep 4, 2017 at 4:15 PM, Tom Lane wrote: > Also, the main thing that we need xact.c's involvement for in the first > place is the fact that implicit transaction blocks, unlike regular ones, > auto-cancel on an error, leaving you outside a block not inside a failed >

Re: [HACKERS] strange case of kernel performance regression (4.3.x and newer)

2016-12-05 Thread Catalin Iacob
On Wed, Oct 5, 2016 at 8:18 AM, Tomas Vondra wrote: > Over the past couple of days I've been doing a bit of benchmarking for the > "group clog" patch [1], and I've ran into what I suspect might be a fairly > serious performance regression on newer kernels

Re: [HACKERS] Patch: Implement failover on libpq connect level.

2016-11-23 Thread Catalin Iacob
On Tue, Nov 22, 2016 at 8:38 AM, Tsunakawa, Takayuki wrote: > transaction_read_only=on does not mean the standby. As the manual article on > hot standby says, they are different. > I'm afraid that if the current patch is committed, you will lose interest in >

Re: [HACKERS] Patch: Implement failover on libpq connect level.

2016-11-16 Thread Catalin Iacob
On Wed, Nov 16, 2016 at 2:50 PM, Robert Haas wrote: > Hmm, let's go back to the JDBC method, then. "show > transaction_read_only" will return true on a standby, but presumably > also on any other non-writable node. You could even force it to be > true artificially if you

Re: [HACKERS] Patch: Implement failover on libpq connect level.

2016-11-15 Thread Catalin Iacob
On Tue, Nov 15, 2016 at 3:42 PM, Alvaro Herrera wrote: > FWIW I'm not sure "primary" is the right term here either. I think what > we really want to know is whether the node can accept writes; maybe > "pg_is_writable_node". This made me think of another complication:

Re: [HACKERS] Patch: Implement failover on libpq connect level.

2016-11-15 Thread Catalin Iacob
On Tue, Nov 15, 2016 at 5:58 PM, Robert Haas wrote: > On Tue, Nov 15, 2016 at 9:42 AM, Alvaro Herrera > wrote: >> I would rather come up with something that works in both cases that we >> can extend internally later, say pg_is_primary_node() or

Re: [HACKERS] [COMMITTERS] pgsql: Add TAP tests for pg_dump

2016-05-09 Thread Catalin Iacob
On 5/9/16, Stephen Frost wrote: >> And what if the tests are buggy? New test suites should go through a >> CF first I think for proper review. And this is clearly one. > > They still won't result in data loss, corruption, or other direct impact > on our users, even if they are

Re: [HACKERS] proposal: PL/Pythonu - function ereport

2016-03-01 Thread Catalin Iacob
On 3/1/16, Pavel Stehule wrote: >> I though about it before and I prefer variant with possibility to enter >> message as keyword parameter. That's also ok, but indeed with a check that it's not specified twice which I see you already added. > I merged your patches

Re: [HACKERS] proposal: PL/Pythonu - function ereport

2016-02-29 Thread Catalin Iacob
On 2/26/16, Pavel Stehule wrote: > Sending updated version I did some changes on top of your last version as that was easier than commenting about them, see attached. 0001 and 0005 are comment changes. 0002 is really needed, without it the tests fail on Python2.4.

Re: [HACKERS] proposal: PL/Pythonu - function ereport

2016-02-24 Thread Catalin Iacob
On Fri, Feb 19, 2016 at 9:41 PM, Pavel Stehule wrote: > It looks like good idea. Last version are not breaking compatibility - and > I think so it can works. > > I wrote the code, that works on Python2 and Python3 Hi, I've attached a patch on top of yours with some

Re: [HACKERS] proposal: make NOTIFY list de-duplication optional

2016-02-21 Thread Catalin Iacob
On Sat, Feb 20, 2016 at 2:00 PM, Filip Rembiałkowski wrote: > I was stuck because both syntaxes have their ugliness. NOTIFY allows the > payload to be NULL: > NOTIFY chan01; > > How would this look like in "never" mode? > NOTIFY chan01, NULL, 'never'; -- seems very

Re: [HACKERS] proposal: make NOTIFY list de-duplication optional

2016-02-19 Thread Catalin Iacob
On 2/9/16, Tom Lane wrote: > FWIW, I think it would be a good thing if the NOTIFY statement syntax were > not remarkably different from the syntax used in the pg_notify() function > call. To do otherwise would certainly be confusing. So on the whole > I'd go with the "NOTIFY

Re: [HACKERS] proposal: PL/Pythonu - function ereport

2016-02-18 Thread Catalin Iacob
On 2/18/16, Pavel Stehule wrote: > it doesn't look badly. Is there any possibility how to emulate it with > Python2 ? What do you think about some similar implementation on Python2? The example code I gave works as is in Python2. The Python3 keyword only arguments are

Re: [HACKERS] proposal: PL/Pythonu - function ereport

2016-02-17 Thread Catalin Iacob
On Wed, Feb 17, 2016 at 3:32 PM, Pavel Stehule wrote: >> Python 3 has keyword only arguments. It occurs to me they're exactly >> for "optional extra stuff" like detail, hint etc. >> Python 2 doesn't have that notion but you can kind of fake it since >> you get an args

Re: [HACKERS] proposal: PL/Pythonu - function ereport

2016-02-16 Thread Catalin Iacob
After I unzip the patch it doesn't apply. patch says: patch: Only garbage was found in the patch input. It's a combined diff, the git-diff manual says this about it: Chunk header format is modified to prevent people from accidentally feeding it to patch -p1. Combined diff format was created

Re: [HACKERS] proposal: PL/Pythonu - function ereport

2016-02-16 Thread Catalin Iacob
On Tue, Feb 16, 2016 at 5:18 PM, Catalin Iacob <iacobcata...@gmail.com> wrote: > We have > elog_test_legacy to test elog_test_legacy=true, the rest of the tests > should use legacy_custom_exception=false. This should have been: We have elog_test_legacy to test legacy_custom

Re: [HACKERS] proposal: PL/Pythonu - function ereport

2016-02-16 Thread Catalin Iacob
On Sat, Feb 13, 2016 at 4:26 PM, Pavel Stehule wrote: > I am sending new version. Current version does: Hello, I had a look and I like the big picture. Tests fail when built against Python 3.5 with stuff like this in regression.diffs: -ERROR: plpy.Error: stop on error

Re: [HACKERS] proposal: PL/Pythonu - function ereport

2016-02-04 Thread Catalin Iacob
On Tue, Feb 2, 2016 at 11:52 PM, Alvaro Herrera wrote: > Robert Haas wrote: >> The eventual committer is likely to be much happier with this patch if >> you guys have achieved consensus among yourselves on the best >> approach. > > Actually I imagine that if there's no

Re: [HACKERS] proposal: PL/Pythonu - function ereport

2016-02-02 Thread Catalin Iacob
On Tue, Feb 2, 2016 at 3:48 PM, Pavel Stehule wrote: > If we decided to break compatibility, then we have to do explicitly. Thid > discussion can continue with commiter, but send path with duplicitly defined > functions has not sense for me. Currently I out of office, so

Re: [HACKERS] proposal: PL/Pythonu - function ereport

2016-02-01 Thread Catalin Iacob
On Mon, Feb 1, 2016 at 5:37 PM, Pavel Stehule <pavel.steh...@gmail.com> wrote: > Dne 29. 1. 2016 18:09 napsal uživatel "Catalin Iacob" > <iacobcata...@gmail.com>: >> Looking at the output above, I don't see who would rely on calling >> plpy.error with mul

Re: [HACKERS] proposal: PL/Pythonu - function ereport

2016-01-29 Thread Catalin Iacob
On Tue, Jan 26, 2016 at 5:42 PM, Pavel Stehule wrote: > I removed from previous patch all OOP related changes. New patch contains > raise_ functions only. This interface is new generation of previous > functions: info, notice, warning, error with keyword parameters

Re: [HACKERS] proposal: PL/Pythonu - function ereport

2016-01-25 Thread Catalin Iacob
On 1/21/16, Pavel Stehule <pavel.steh...@gmail.com> wrote: > 2016-01-14 17:16 GMT+01:00 Catalin Iacob <iacobcata...@gmail.com>: >> Consider this call chain: SQL code 1 -> PL/Python code 1 -> SPI (via >> plpy.execute) -> SQL code 2 -> PL/Python code 2 >

Re: [HACKERS] proposal: PL/Pythonu - function ereport

2016-01-14 Thread Catalin Iacob
On Wed, Jan 13, 2016 at 7:40 PM, Jim Nasby <jim.na...@bluetreble.com> wrote: > On 1/12/16 11:25 AM, Catalin Iacob wrote: >> They're similar but not really the same thing. raise Error and >> plpy.error are both ways to call ereport(ERROR, ...) while SPIError is >> ra

Re: [HACKERS] proposal: PL/Pythonu - function ereport

2016-01-12 Thread Catalin Iacob
On Tue, Jan 12, 2016 at 5:50 PM, Pavel Stehule wrote: > Error and Fatal exception classes are introduced in my patch - it was Peter' > request (if I remember it well), and now I am thinking so it is not good > idea. Now everybody is confused :). Your patch does: -

Re: [HACKERS] proposal: PL/Pythonu - function ereport

2016-01-12 Thread Catalin Iacob
On Mon, Jan 11, 2016 at 7:33 PM, Pavel Stehule wrote: > I see it. > > it looks like distinguish between Error and SPIError is wrong way. And I > have any other ugly use case. > > If I raise a Error from one PLPythonu function, then in other PLPython > function I'll trap a

Re: [HACKERS] proposal: PL/Pythonu - function ereport

2016-01-11 Thread Catalin Iacob
On Fri, Jan 8, 2016 at 7:56 AM, Pavel Stehule wrote: >> 3. PLy_error__init__ is used for BaseError but always sets an >> attribute called spidata, I would expect that to only happen for >> SPIError not for BaseError. You'll need to pick some other way of >> attaching the

Re: [HACKERS] proposal: PL/Pythonu - function ereport

2016-01-07 Thread Catalin Iacob
shellOn Thu, Dec 31, 2015 at 11:37 AM, Pavel Stehule wrote: > here is new version. And here's a new review, sorry for the delay. > Now I use a common ancestor "plpy.BaseError" for plpy builtin classes. So > plpy.SPIError isn't descendant of plpy.Error and then there are

Re: [HACKERS] No Issue Tracker - Say it Ain't So!

2016-01-06 Thread Catalin Iacob
On Wed, Jan 6, 2016 at 3:11 AM, Jim Nasby wrote: > Which doesn't help anyone, because neither of those provide a list of "hey, > here's stuff you could do to contribute". The closest we come to that is the > TODO, which isn't well known and has almost no items for

Re: [HACKERS] proposal: PL/Pythonu - function ereport

2015-12-07 Thread Catalin Iacob
On Thu, Dec 3, 2015 at 6:54 PM, Pavel Stehule wrote: > Don't understand - if Fatal has same behave as Error, then why it cannot be > inherited from Error? > > What can be broken? Existing code that did "except plpy.Error as exc" will now also be called if plpy.Fatal is

Re: [HACKERS] proposal: multiple psql option -c

2015-12-04 Thread Catalin Iacob
On Fri, Dec 4, 2015 at 3:47 PM, Robert Haas wrote: > For the most part, the cleanups in this version are just cosmetic: I > fixed some whitespace damage, and reverted some needless changes to > the psql references page that were whitespace-only adjustments. In a > few

Re: [HACKERS] proposal: PL/Pythonu - function ereport

2015-12-03 Thread Catalin Iacob
On Thu, Dec 3, 2015 at 7:58 AM, Pavel Stehule wrote: > I am sorry, I don't understand. Now due inheritance plpy.Fatal and > plpy.SPIError has availability to use keyword parameters. Indeed, I didn't realize this, but I don't think it changes the main argument. What I

Re: [HACKERS] proposal: PL/Pythonu - function ereport

2015-12-02 Thread Catalin Iacob
On Tue, Dec 1, 2015 at 2:17 AM, Pavel Stehule wrote: > here is complete patch - regress tests for all supported Python branches I had a look at what changed in v10 since my last reviewed version and indeed most of it is straightforward: renames from SPIError to Error.

Re: [HACKERS] proposal: multiple psql option -c

2015-12-01 Thread Catalin Iacob
On Tue, Dec 1, 2015 at 1:53 PM, Michael Paquier wrote: > Attached is a patch implementing those suggestions. This simplifies > the code without changing its usefulness. If you are fine with those > changes I will switch this patch as ready for committer. I tested the

Re: [HACKERS] proposal: multiple psql option -c

2015-11-25 Thread Catalin Iacob
On Wed, Nov 18, 2015 at 5:49 PM, Catalin Iacob <iacobcata...@gmail.com> wrote: > On Tue, Nov 17, 2015 at 10:13 PM, Tom Lane <t...@sss.pgh.pa.us> wrote: >> 1. -c no longer implies --no-psqlrc. That's a backwards incompatibility, >> but very easy to explain and very e

Re: [HACKERS] proposal: multiple psql option -c

2015-11-18 Thread Catalin Iacob
On Tue, Nov 17, 2015 at 10:13 PM, Tom Lane wrote: > 1. -c no longer implies --no-psqlrc. That's a backwards incompatibility, > but very easy to explain and very easy to work around. > > 2. You can have multiple -c and/or -f. Each -c is processed in > the traditional way, ie,

Re: [HACKERS] proposal: multiple psql option -c

2015-11-16 Thread Catalin Iacob
On Sun, Nov 15, 2015 at 3:53 PM, Andrew Dunstan wrote: > I suggest you review the original thread on this subject before a line was > ever written. "multiple" (see subject line on this whole thread) is clearly > what is being asked for. Making people turn that into a single

Re: [HACKERS] proposal: multiple psql option -c

2015-11-15 Thread Catalin Iacob
On Sun, Nov 15, 2015 at 1:27 AM, Andrew Dunstan wrote: > That seems to me to get rid of the main motivation for this change, which is > to allow multiple such arguments, which together would as as if they were > all written to a file which was then invoked like -f file. It

Re: [HACKERS] proposal: multiple psql option -c

2015-11-13 Thread Catalin Iacob
So I promised I'd try to document this. I had a look at the proposed semantics of -C and I think in the patch they're too complicated which makes explaining them hard. My assumptions about behaviour without this patch, from reading the docs and some experimenting, correct me if I'm wrong: 1.

Re: [HACKERS] proposal: PL/Pythonu - function ereport

2015-11-09 Thread Catalin Iacob
On Mon, Nov 9, 2015 at 6:31 PM, Pavel Stehule wrote: > merged your patch So, I just that tested version 08 is the same as the previous patch + my change and I already tested that on Python 2.4, 2.5, 2.6, 2.7 and 3.5. All previous comments addressed so I'll mark this

Re: [HACKERS] proposal: PL/Pythonu - function ereport

2015-11-09 Thread Catalin Iacob
On Mon, Nov 9, 2015 at 2:58 PM, Pavel Stehule wrote: > I needed to understand the support for Python 3.5. > > The patch with the fix is attached regress tests 3.5 Python I wanted to type a reply this morning to explain and then I noticed there's another file (_0.out) for

Re: [HACKERS] proposal: multiple psql option -c

2015-11-05 Thread Catalin Iacob
On Thu, Nov 5, 2015 at 5:27 PM, Robert Haas wrote: >> I wrote some text. But needs some work of native speaker. > > It does. It would be nice if some kind reviewer could help volunteer > to clean that up. I'll give it a go sometime next week. -- Sent via pgsql-hackers

Re: [HACKERS] proposal: PL/Pythonu - function ereport

2015-11-04 Thread Catalin Iacob
On Wed, Nov 4, 2015 at 10:12 AM, Pavel Stehule wrote: > It helped me lot of, thank you Welcome, I learned quite some from the process as well. >> >> >> There's just the doc part left then. > > > done We're almost there but not quite. There's still a typo in the docs:

Re: [HACKERS] proposal: PL/Pythonu - function ereport

2015-11-03 Thread Catalin Iacob
Sorry, you're right, I didn't notice the x = plpy.SPIError() test. I did notice that you included the kw != NULL, I was explaining why it really is needed even though it *seems* the code also works without it. There's just the doc part left then. -- Sent via pgsql-hackers mailing list

Re: [HACKERS] proposal: PL/Pythonu - function ereport

2015-11-03 Thread Catalin Iacob
On Tue, Nov 3, 2015 at 12:49 PM, Pavel Stehule wrote: >> 1. in PLy_spi_error__init__ you need to check kw for NULL before doing >> PyDict_Size(kw) otherwise for plpy.SPIError() you get Bad internal >> call because PyDict_Size expects a real dictionary not NULL > > >

Re: [HACKERS] proposal: PL/Pythonu - function ereport

2015-11-02 Thread Catalin Iacob
Hello, Here's a detailed review: 1. in PLy_spi_error__init__ you need to check kw for NULL before doing PyDict_Size(kw) otherwise for plpy.SPIError() you get Bad internal call because PyDict_Size expects a real dictionary not NULL 2. a test with just plpy.SPIError() is still missing, this would

Re: [HACKERS] proposal: PL/Pythonu - function ereport

2015-10-28 Thread Catalin Iacob
On Tue, Oct 27, 2015 at 9:34 AM, Pavel Stehule <pavel.steh...@gmail.com> wrote: > Hi > > 2015-10-23 7:34 GMT+02:00 Catalin Iacob <iacobcata...@gmail.com>: >> The current code doesn't build on Python3 because the 3rd argument of >> PyMethod_New, the troubled

Re: [HACKERS] proposal: PL/Pythonu - function ereport

2015-10-22 Thread Catalin Iacob
On Sat, Oct 17, 2015 at 8:18 PM, Pavel Stehule wrote: > here is new patch > > cleaned, all unwanted artefacts removed. I am not sure if used way for > method registration is 100% valid, but I didn't find any related > documentation. Pavel, some notes about the patch, not

Re: [HACKERS] Adding since-version tags to the docs?

2015-09-01 Thread Catalin Iacob
On Mon, Aug 31, 2015 at 4:48 PM, Tom Lane wrote: > Right now, you might well care about whether a feature arrived in 9.3 vs > 9.4, for instance; but it's highly unlikely that you care whether a > feature arrived in 7.1 or 7.2. The problem with this proposal is that > it will

Re: [HACKERS] jsonb concatenate operator's semantics seem questionable

2015-05-18 Thread Catalin Iacob
On Mon, May 18, 2015 at 9:03 PM Andrew Dunstan and...@dunslane.net wrote: So you're arguing that we shouldn't call the operation in question || because it's pretty much the same, mutatis mutandis, as the hstore operation of the same name. You've lost me. Hopefully this helps. Peter's