Opened 17 years ago
Closed 12 years ago
#7140 closed Bug (wontfix)
Errors in escaping fieldnames in Oracle
Reported by: | Owned by: | Matt Boersma | |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | qsrf-cleanup oracle, quote, escape |
Cc: | herwin@…, Erin Kelly | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | yes | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
According to the comments it's better to have your fieldnames in oracle quoted, but in our case this yielded to errors because the databasename had to be included in the tablename, which means all parts (database, table, field) have to be quoted seperately. The following snippet was the solution in our case (but I'm not sure if this always works, I'm no oracle expert)
--- db/backends/oracle/base.py (revision 7510) +++ db/backends/oracle/base.py (working copy) @@ -109,6 +109,7 @@ # We simplify things by making Oracle identifiers always uppercase. if not name.startswith('"') and not name.endswith('"'): name = '"%s"' % util.truncate_name(name.upper(), self.max_name_length()) + name = name.replace('.', '"."') return name.upper() def random_function_sql(self):
Change History (16)
comment:1 by , 17 years ago
Cc: | added |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 17 years ago
Keywords: | qsrf-cleanup added |
---|
comment:3 by , 17 years ago
milestone: | → 1.0 |
---|
comment:4 by , 16 years ago
Ian, this is much more your department than mine. There's nothing in the query construction code that is going to care about this. It treats the output of quote_name()
as an opaque string and never tries to introspect it or change it in any way.
Whatever makes you happy is the Right Thing To Do. :-)
comment:5 by , 16 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:6 by , 16 years ago
Owner: | changed from | to
---|---|
Status: | assigned → new |
comment:7 by , 16 years ago
Needs tests: | set |
---|---|
Owner: | changed from | to
Status: | new → assigned |
comment:8 by , 16 years ago
@mboersma: can we get a quick thumbs up / thumbs down on this for 1.0? Is it really a bug, or just a nice-to-have feature to work around a lack of schema support. How practical is it not to include this? Or, if you're really confident that it's a solid fix, can it be checked in so that we know the resolution? The days are running out for people to have time to test things.
comment:9 by , 16 years ago
I've been reviewing this, and I can say at least the patch does no harm--no tests break. But I've been trying to create a test case to see that it actually fixes the stated problem: that specifying db_table = '"schema.tablename"' works ok. My approach was to create a queryset, call qs.query.as_sql(), then change the Model._meta.db_table, create a new queryset and call qs.query.as_sql again. (It's a finicky test, but I special-cased it only to run on Oracle in the queries test suite.)
Unfortunately, in the second test I get something like:
SELECT "schema.tablename"."colname" FROM ....
which Oracle won't like.
So I'm not currently able to verify it fixes the problem. And given that the proper solution awaits in #6148 as Ian Kelly mentions, I'll have to say thumbs down for now.
(But I have plenty of time this weekend to attend to Django, so if someone has a strong argument for thumbs up, I will be glad to dig into this tomorrow.)
comment:10 by , 16 years ago
milestone: | 1.0 → post-1.0 |
---|
In light of Matt's comment, let's move the milestone to get it off the showstopper list and it can be moved back if good arguments / working code appear.
comment:12 by , 16 years ago
The proposed patch works fine here (with a legacy database, using db_table). Since it does not break anything and #6148 is still pending, is there any chance to have this patch for 1.1?
comment:13 by , 14 years ago
Severity: | → Normal |
---|---|
Type: | → Bug |
comment:16 by , 12 years ago
Resolution: | → wontfix |
---|---|
Status: | assigned → closed |
Im going to close this one. One can already use db_table = '"SOME_SCHEMA.SOME_TABLE"' if they want to. This ticket asks to allow usage of 'SOME_SCHEMA.SOME_TABLE'. No point in my opinion.
The reason this wasn't done before was that it caused havoc with join aliases, which were generated by munging the table name. Now that queryset-refactor is merged into trunk, that's no longer an issue, so this may be a viable temporary fix until somebody has the time to work on implementing general schema support in #6148.
This solution doesn't work for doing cross-schema introspection, however. There's also a very minor backward incompatibility if anybody is actually using periods in their table names.