2015-12-08 7:06 GMT+01:00 Catalin Iacob <iacobcata...@gmail.com>:

> On Thu, Dec 3, 2015 at 6:54 PM, Pavel Stehule <pavel.steh...@gmail.com>
> 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 raised. That wasn't the case so this changes
> meaning of existing code, therefore it introduces an incompatibility.

yes, there should be some common ancestor for plpy.Error and plpy.Fatal

Have you any idea, how this ancestor should be named?

> >>
> >> 5. all the reporting functions: plpy.debug...plpy.fatal functions
> >> should also be changed to allow more arguments than the message and
> >> allow them as keyword arguments
> >
> >
> > this is maybe bigger source of broken compatibility
> >
> > lot of people uses plpy.debug(var1, var2, var3)
> >
> > rich constructor use $1 for message, $2 for detail, $3 for hint. So it
> was a
> > reason, why didn't touch these functions
> No, if you use PyArg_ParseTupleAndKeywords you'll have Python accept
> all of these so previous ways are still accepted:
> plpy.error('a_msg', 'a_detail', 'a_hint')
> plpy.error'a_msg', 'a_detail', hint='a_hint')
> plpy.error('a_msg', detail='a_detail', hint='a_hint')
> plpy.error(msg='a_msg', detail='a_detail', hint='a_hint')
> plpy.error('a_msg')
> plpy.error(msg='a_msg')
> etc.
> But I now see that even though the docs say plpy.error and friends
> take a single msg argument, they actually allow any number of
> arguments. If multiple arguments are passed they are converted to a
> tuple and the string representation of that tuple goes into
> ereport/plpy.Error:
>   RETURNS int
> AS $$
>   try:
>     plpy.error('an error message', 'detail for error', 'hint for error')
>   except plpy.Error as exc:
>     plpy.info('have exc {}'.format(exc))
>     plpy.info('have exc.args |{}| of type {}'.format(exc.args,
> type(exc.args)))
> $$ LANGUAGE plpython3u;
> SELECT test();
> [catalin@archie pg]$ psql -f plpy test
> psql:plpy:13: INFO:  have exc ('an error message', 'detail for error',
> 'hint for error')
> psql:plpy:13: INFO:  have exc.args |("('an error message', 'detail for
> error', 'hint for error')",)| of type <class 'tuple'>
> In the last line note that exc.args (the args tuple passed in the
> constructor of plpy.Error) is a tuple with a single element which is a
> string which is a representation of the tuple passed into plpy.error.
> Don't know why this was thought useful, it was certainly surprising to
> me. Anyway, the separate $2, $3 etc are currently not detail and hint,
> they're just more stuff that gets appended to this tuple. They don't
> get passed to clients as hints. And you can pass lots of them not just
> detail and hint.

using tuple is less work for you, you don't need to concat more values into
one. I don't know, how often this technique is used - probably it has sense
only for NOTICE and lower levels - for adhoc debug messages. Probably not
used elsewhere massively.

> My proposal would change the meaning of this to actually interpret the
> second argument as detail, third as hint and to only allow a limited
> number (the ones with meaning to ereport). The hint would go to
> ereport so it would be a real hint: it would go to clients as HINT and
> so on. I think that's way more useful that what is done now. But I now
> see my proposal is also backward incompatible.

It was my point

> > I am not against to plpy.ereport function - it can live together with
> rich
> > exceptions class, and users can use what they prefer.
> I wasn't suggesting introducing ereport, I was saying the existing
> functions and exceptions are very similar to your proposed ereport.
> Enhancing them to take keyword arguments would make them a bit more
> powerful. Adding ereport would be another way of doing the same thing
> so that's more confusing than useful.


> All in all it's hard to improve this cleanly. I'm still not sure the
> latest patch is a good idea but now I'm also not sure what I proposed
> is better than leaving it as it is.

We can use different names, we should not to implement all changes at once.

My main target is possibility to raise rich exception instead dirty
workaround. Changing current functions isn't necessary - although if we are
changing API, is better to do it once.



Reply via email to