Opened 10 years ago
Closed 7 years ago
#23546 closed New feature (fixed)
Add kwargs support for cursor.callproc()
Reported by: | fatal10110 | Owned by: | Mariusz Felisiak |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | oracle |
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
I think it will be useful to add kwargs or args parameters to callproc method for those backends that support more parameters for callproc method (e.g cx_oracle)
I can not use named parameters for my procedures with django db module. =
Change History (19)
comment:1 by , 10 years ago
Keywords: | oracle added |
---|---|
Needs documentation: | set |
Needs tests: | set |
comment:2 by , 10 years ago
Needs documentation: | unset |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:3 by , 10 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:4 by , 10 years ago
Has patch: | set |
---|
pull request created
https://github.com/django/django/pull/3342
already tested on ubuntu 12.04 server(amd64) with oracle XE 11g
comment:5 by , 10 years ago
Needs tests: | unset |
---|
follow-up: 8 comment:7 by , 10 years ago
Needs documentation: | set |
---|---|
Patch needs improvement: | set |
Hi,
I made some notes on the PR. Additionally, documentation needed. I suggested making the feature more general, less Oracle-specific, but even if this path is not taken, the new behavior needs to be documented. Currently, callproc is not documented at all, which is a shame, but forgivable considering that the function is as defined in PEP 249; deviating from the PEP justifies adding explicit documentation, probably in docs/topics/db/sql.txt
; whether or not that is done, the addition should be mentioned in the release notes (docs/releases/1.8.txt
).
comment:8 by , 10 years ago
Needs documentation: | unset |
---|---|
Patch needs improvement: | unset |
Replying to shaib:
Hi,
I made some notes on the PR. Additionally, documentation needed. I suggested making the feature more general, less Oracle-specific, but even if this path is not taken, the new behavior needs to be documented. Currently, callproc is not documented at all, which is a shame, but forgivable considering that the function is as defined in PEP 249; deviating from the PEP justifies adding explicit documentation, probably in
docs/topics/db/sql.txt
; whether or not that is done, the addition should be mentioned in the release notes (docs/releases/1.8.txt
).
Thanks for your detailed comments. New commit pushed. I also replied your comments on github.
follow-up: 10 comment:9 by , 10 years ago
Patch needs improvement: | set |
---|
Implementation needs a DatabaseFeature (e.g. callproc_supports_kwargs
) to explicitly enable support for kparams for a backend. The current patch will pass along kparams to the underlying backend as long as they are provided to callproc, which will raise a confusing exception to the user. It should probably raise django.db.utils.NotSupportedError
.
comment:10 by , 10 years ago
Patch needs improvement: | unset |
---|
Replying to manfre:
Implementation needs a DatabaseFeature (e.g.
callproc_supports_kwargs
) to explicitly enable support for kparams for a backend. The current patch will pass along kparams to the underlying backend as long as they are provided to callproc, which will raise a confusing exception to the user. It should probably raisedjango.db.utils.NotSupportedError
.
Thanks. Made the changes you suggested.
comment:11 by , 10 years ago
I've added some Notes on the PR, and opened a related mailing list discussion.
comment:12 by , 10 years ago
Easy pickings: | unset |
---|---|
Patch needs improvement: | set |
comment:13 by , 9 years ago
Owner: | removed |
---|---|
Status: | assigned → new |
comment:14 by , 8 years ago
Is this patch still being considered for release?
I have the same use-case as the ticket description(cx_Oracle keywordParameters).
comment:15 by , 8 years ago
If you'd like to help, please update the old pull request for the comments.
comment:16 by , 8 years ago
Has patch: | unset |
---|---|
Owner: | set to |
Patch needs improvement: | unset |
Status: | new → assigned |
Version: | 1.7 → master |
comment:18 by , 7 years ago
Summary: | callproc **kwargs or *args parameter → Add kwargs support for cursor.callproc() |
---|---|
Triage Stage: | Accepted → Ready for checkin |
PEP 249 says that callproc only accept positional arguments.
That said, since we're just forwarding arguments to the underlying library, this small deviation seems acceptable, with a comment explaining it.