cursor.callproc()

[Shai Berger]

Hi all,

For a very long time -- as far as I'm aware, forever -- we've had, in our
cursor classes, a "callproc()" method that follows the callproc() definition of
pep249[1]; this allows database stored procedures to be called.

Recently, we've had a ticket[2] and PR[3] to enhance this method -- the pep249
definition takes the procedure name and a list of positional arguments; some
database drivers (notably cx_Oracle) allow passing also a dictionary of named
arguments, so we want to support that too. While reviewing the PR, we found
that

(a) cursor.callproc is not documented
(b) cursor.callproc is not tested (except for one Oracle-specific test that
uses it to test something else)

Which means, essentially, that it is treated as an intenal API.

I see no reason that we shouldn't make it public API -- but

(a) I might be wrong
(b) If we do, then we need to document it and test it on all supported
backends (which implement it). I am not aware of any "universally available"
stored procedure nor even universally valid syntax for creating stored
procedures; so this probably amounts to backend-specific tests.

So, I'm asking for advice.

If there are no objections, I will open a ticket calling for the above, and
try to coordinate the work on it with the work on #23546.

Have fun,
Shai.

[1] http://legacy.python.org/dev/peps/pep-0249/#callproc
[2] https://code.djangoproject.com/ticket/23546
[3] https://github.com/django/django/pull/3342

[Marc Tamlyn]

I guess now with migrations we have a nice way of running the SQL against the database to create the stored procedures.

However if we plan to make this a public API, it should be a nice one. Something along the lines of db.procedures.proc_name(*args, **kwargs) would be preferable I think. Obviously this requires more magic to make it work (or explicit registration of your procedures).

Marc

--
You received this message because you are subscribed to the Google Groups "Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/201410190225.34849.shai%40platonix.com.
For more options, visit https://groups.google.com/d/optout.

[Shai Berger]

Hi Marc and all,

On Sunday 19 October 2014 09:54:39 Marc Tamlyn wrote:
> I guess now with migrations we have a nice way of running the SQL against
> the database to create the stored procedures.
>

I'm not sure RunSQL is much preferable to cursor.execute() in this context.
And either way, it doesn't solve the basic issue: The SQL to run would still
differ between backends, so tests would still need to be backend-specific.

> However if we plan to make this a public API, it should be a nice one.
> Something along the lines of db.procedures.proc_name(*args, **kwargs) would
> be preferable I think. Obviously this requires more magic to make it work
> (or explicit registration of your procedures).
>

That would be a very nice API indeed, but it is much higher-level than the one
under discussion. At issue is a callproc() that implements pep 249, much like
its siblings execute(), fetchone() etc; that is why I am not even suggesting
to modify it to the obvious callproc(procname, *args, **kwargs).
Cursor.execute() is documented, public API -- are there reasons to deny
callproc() the same status?

Shai.

[Marc Tamlyn]

I was thinking in the context of a project - creating a procedure to use in the code. Naturally cursor.execute() is perfectly fine for tests, which would need to be backend dependent.

I can't think of a reason to deny cursor.callproc() the same status as cursor.execute().

Marc

--
You received this message because you are subscribed to the Google Groups "Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/201410200012.08958.shai%40platonix.com.

[Shai Berger]

On Monday 20 October 2014 00:38:52 Marc Tamlyn wrote:
> I was thinking in the context of a project - creating a procedure to use in
> the code.

Oh, I see. Your API suggestion is also perfect for that use.

> Naturally cursor.execute() is perfectly fine for tests, which
> would need to be backend dependent.
>
> I can't think of a reason to deny cursor.callproc() the same status as
> cursor.execute().
>

Thanks,
Shai.

[Carl Meyer]

Hi Marc,

On 10/19/2014 12:54 AM, Marc Tamlyn wrote:
> I guess now with migrations we have a nice way of running the SQL
> against the database to create the stored procedures.
>
> However if we plan to make this a public API, it should be a nice one.
> Something along the lines of db.procedures.proc_name(*args, **kwargs)
> would be preferable I think. Obviously this requires more magic to make
> it work (or explicit registration of your procedures).

I know this is hypothetical, but I don't think that is a particularly
nicer API, or that we should provide such syntactic sugar atop
callproc(). Providing the procedure name as a string is not really a
problem, and is preferable to doing `__getattr__` magic or requiring
registration of procedures; the syntactic sugar just doesn't provide
enough benefit to justify the magic, and all the various ways that that
magic could confuse users and cause maintenance issues.

Carl

[Carl Meyer]

(I should clarify: I do think there are ways that we could improve on
the PEP 249 callproc() API if we were providing a higher-level
Django-specific alternative. I just don't think that moving the
procedure name from a string argument to a magical getattr-based Python
function is a good idea.)

Carl

[Chris Foresman]

Is there some benefit to using `.callproc()` over this?

``` python

    query = 'CALL sp_recommendation_engine(%s, %s)'

    profile = user.get_profile()

    cursor = connection.cursor()

    cursor.execute(query, [user.id, profile.id])

```

[Shai Berger]

On Tuesday 21 October 2014 18:23:44 Chris Foresman wrote:
> Is there some benefit to using `.callproc()` over this?
>
> ``` python
> query = 'CALL sp_recommendation_engine(%s, %s)'
> profile = user.get_profile()
> cursor = connection.cursor()
> cursor.execute(query, [user.id, profile.id])
> ```
>

There are two benefits:

1) Unlike Python functions, SQL stored procedures can have output parameters
(and input/output parameters). callproc() allows you to get these (it returns
the sequence of parameters, with outputs placed appropriately), while
execute() can only return the procedure's return value (I'm not quite sure
about procedures yielding result-sets -- callproc() will let you fetch the
results, I'm not sure the same holds for execute() universally).

2) Surprisingly, stored procedure invocation syntax is not completely
standard; for example, on PostgreSQL, you should have

query= 'SELECT sp_recommendation_engine(%s, %s)'

HTH,
Shai.

[Shai Berger]

I respectfully disagree. The kind of "magic" Marc suggested lets you, as a
user. treat procedure calls as function calls; that is a very natural thing to
do. I'd argue that in the common case, the user shouldn't care if the function
they are calling is implemented in Python or Procedural SQL (assuming it is
going to interact with the database either way), and so it is good API design
to abstact this detail away.

(I encourage everyone to take a look at plumbum[1], which, among other things,
allows you to import shell commands into your namespace using a similar
abstraction. This is actually not my favorite part of that library -- that
would be plumbum.cli[2] -- but when you want to write system scripts, it makes
things really sweet; you get the software-engineering benefits of Python, with
the shell's straightforward command invocation).

Shai

[1] http://plumbum.readthedocs.org, https://github.com/tomerfiliba/plumbum
[2] http://plumbum.readthedocs.org/en/latest/cli.html

[Chris Foresman]

Good points. I went looking for `callproc()` documentation and couldn't find anything useful, which if I understand correctly is part of the reason this thread started. So +1 on documenting it!

The most complicated part of dealing with store procedures from a high level is getting them in the database to begin with, and updating them if they need to change. Obviously I'm just manually entering them via the console, but it would be great if there was some consistent way to add them to the database via Migrations or something similar.

[Carl Meyer]

Hi Chris,

On 10/22/2014 09:05 AM, Chris Foresman wrote:
> Good points. I went looking for `callproc()` documentation and couldn't
> find anything useful, which if I understand correctly is part of the
> reason this thread started. So +1 on documenting it!
>
> The most complicated part of dealing with store procedures from a high
> level is getting them in the database to begin with, and updating them
> if they need to change. Obviously I'm just manually entering them via
> the console, but it would be great if there was some consistent way to
> add them to the database via Migrations or something similar.

As Marc suggested, I think migrations do already solve this problem, via
the RunSQL operation. (It doesn't offer any abstraction over writing the
stored procedure, but I'm not sure what kind of abstraction would be
useful there. It does provide a place to put it that ensures it will get
run on every database - though previously you could use initial SQL for
that purpose.)

Carl

[Carl Meyer]

Hi Shai,

On 10/21/2014 04:04 PM, Shai Berger wrote:
> On Monday 20 October 2014 21:26:50 Carl Meyer wrote:
>> Hi Marc,
>>
>> On 10/19/2014 12:54 AM, Marc Tamlyn wrote:
>>> I guess now with migrations we have a nice way of running the SQL
>>> against the database to create the stored procedures.
>>>
>>> However if we plan to make this a public API, it should be a nice one.
>>> Something along the lines of db.procedures.proc_name(*args, **kwargs)
>>> would be preferable I think. Obviously this requires more magic to make
>>> it work (or explicit registration of your procedures).
>>
>> I know this is hypothetical, but I don't think that is a particularly
>> nicer API, or that we should provide such syntactic sugar atop
>> callproc(). Providing the procedure name as a string is not really a
>> problem, and is preferable to doing `__getattr__` magic or requiring
>> registration of procedures; the syntactic sugar just doesn't provide
>> enough benefit to justify the magic, and all the various ways that that
>> magic could confuse users and cause maintenance issues.
>>
> I respectfully disagree. The kind of "magic" Marc suggested lets you, as a
> user. treat procedure calls as function calls; that is a very natural thing to
> do.

Yes, I understand the attraction.

> I'd argue that in the common case, the user shouldn't care if the function
> they are calling is implemented in Python or Procedural SQL (assuming it is
> going to interact with the database either way), and so it is good API design
> to abstact this detail away.

Here is where we differ. I think calling a database stored procedure is
fundamentally a different thing from calling a Python function (even one
that accesses the database), and the difference should be obvious in the
calling code. I don't think it is good API design to abstract away
differences that the caller should be aware of. (For one example of a
difference, database stored procedures "return a modified copy of the
input arguments", which would be extremely unusual behavior for a normal
Python function.)

I think this is mostly a question of level. In an actual Django app that
uses stored procedures, I think it is quite likely a good idea for the
author of the app to provide a nice Python API that abstracts away the
internal implementation. But I think this API should be consciously
designed for-purpose (for example, it should likely return something
other than a possibly-modified copy of all its input arguments), and at
the level of Django core the benefits of a generic API that attempts to
mask the difference between Python functions and database stored
procedures are not enough to justify the magic required.

Perhaps I'm wrong - but I'd definitely want to see the utility of such a
generic "magic" layer proved as an external library before it gets added
to core.

Carl

[Shai Berger]

Hi Carl,

On Wednesday 22 October 2014 19:35:49 Carl Meyer wrote:
> On 10/21/2014 04:04 PM, Shai Berger wrote:
> > I'd argue that in the common case, the user shouldn't care if the
> > function they are calling is implemented in Python or Procedural SQL
> > (assuming it is going to interact with the database either way), and so
> > it is good API design to abstact this detail away.
>
> Here is where we differ. I think calling a database stored procedure is
> fundamentally a different thing from calling a Python function (even one
> that accesses the database), and the difference should be obvious in the
> calling code. I don't think it is good API design to abstract away
> differences that the caller should be aware of. (For one example of a
> difference, database stored procedures "return a modified copy of the
> input arguments", which would be extremely unusual behavior for a normal
> Python function.)
>

That, of course, doesn't have to be the case for the wrapper API.

> I think [...] at

> the level of Django core the benefits of a generic API that attempts to
> mask the difference between Python functions and database stored
> procedures are not enough to justify the magic required.
>
> Perhaps I'm wrong - but I'd definitely want to see the utility of such a
> generic "magic" layer proved as an external library before it gets added
> to core.
>

I agree that this would be the prudent step to take. On second thought, I
expect the benefit to prove quite small, just because people don't use all that
many stored procedures in Django apps.

Shai.

[Michael Manfre]

On Wed, Oct 22, 2014 at 12:35 PM, Carl Meyer <ca...@oddbird.net> wrote:

Here is where we differ. I think calling a database stored procedure is
fundamentally a different thing from calling a Python function (even one
that accesses the database), and the difference should be obvious in the
calling code. I don't think it is good API design to abstract away
differences that the caller should be aware of. (For one example of a
difference, database stored procedures "return a modified copy of the
input arguments", which would be extremely unusual behavior for a normal
Python function.)

I think this is mostly a question of level. In an actual Django app that
uses stored procedures, I think it is quite likely a good idea for the
author of the app to provide a nice Python API that abstracts away the
internal implementation. But I think this API should be consciously
designed for-purpose (for example, it should likely return something
other than a possibly-modified copy of all its input arguments), and at
the level of Django core the benefits of a generic API that attempts to
mask the difference between Python functions and database stored
procedures are not enough to justify the magic required.

Perhaps I'm wrong - but I'd definitely want to see the utility of such a
generic "magic" layer proved as an external library before it gets added
to core.

As someone who has used a very large number of stored procedures with Django, I am a solid -1 on adding a generic "magic" layer to Django. Stored procedures are purpose built. Their python usage should also be purpose built to match the various combinations of input/output parameters, return values, and result sets that are possible for a stored procedure.

Regards,

Michael Manfre

[Carl Meyer]

> On Oct 22, 2014, at 5:56 PM, Shai Berger <sh...@platonix.com> wrote:
>> On Wednesday 22 October 2014 19:35:49 Carl Meyer wrote:
>>> On 10/21/2014 04:04 PM, Shai Berger wrote:
>>> I'd argue that in the common case, the user shouldn't care if the
>>> function they are calling is implemented in Python or Procedural SQL
>>> (assuming it is going to interact with the database either way), and so
>>> it is good API design to abstact this detail away.
>>
>> Here is where we differ. I think calling a database stored procedure is
>> fundamentally a different thing from calling a Python function (even one
>> that accesses the database), and the difference should be obvious in the
>> calling code. I don't think it is good API design to abstract away
>> differences that the caller should be aware of. (For one example of a
>> difference, database stored procedures "return a modified copy of the
>> input arguments", which would be extremely unusual behavior for a normal
>> Python function.)
> That, of course, doesn't have to be the case for the wrapper API.

Well, that's the problem, isn't it? A purpose-built nice Python wrapper for a particular stored procedure can certainly do something better, but I'm not sure what else a generic wrapper system could do. You don't know which parameters to an arbitrary proc might be output parameters (unless you try some kind of runtime introspection, or require a registration step where the user tells you about the signature; at that point you may as well just let the user write their own wrapper). There's a reason 'callproc' has the odd signature it does.

Carl

[Marc Tamlyn]

I will happily cede ground to people who actually use stored procedures.

--
You received this message because you are subscribed to the Google Groups "Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/6F8CC704-DB07-4220-AF85-EBF03BD0141C%40oddbird.net.

[Shai Berger]

Just to give this proper closure: I think there's a lot that can be improved
under certain assumptions -- from the trivial "the user doesn't care about the
output params" to the specific "the procedure yields a result-set that is
suitable for a queryset of model X" (this is the assumption/requirement that
queryset.raw() makes). But given the opposition, I will let this go now.

Shai.