Opened 16 years ago

Closed 12 years ago

#7140 closed Bug (wontfix)

Errors in escaping fieldnames in Oracle

Reported by: herwin@… 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 Erin Kelly, 16 years ago

Cc: Erin Kelly added
Triage Stage: UnreviewedAccepted

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.

comment:2 by George Vilches, 16 years ago

Keywords: qsrf-cleanup added

comment:3 by Jacob, 16 years ago

milestone: 1.0

comment:4 by Malcolm Tredinnick, 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 cmarshal, 16 years ago

Owner: changed from nobody to cmarshal
Status: newassigned

comment:6 by cmarshal, 16 years ago

Owner: changed from cmarshal to nobody
Status: assignednew

comment:7 by Matt Boersma, 16 years ago

Needs tests: set
Owner: changed from nobody to Matt Boersma
Status: newassigned

comment:8 by Malcolm Tredinnick, 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 Matt Boersma, 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 Malcolm Tredinnick, 16 years ago

milestone: 1.0post-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:11 by (none), 15 years ago

milestone: post-1.0

Milestone post-1.0 deleted

comment:12 by aprilmay, 15 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 Luke Plant, 13 years ago

Severity: Normal
Type: Bug

comment:14 by Aymeric Augustin, 12 years ago

UI/UX: unset

Change UI/UX from NULL to False.

comment:15 by Aymeric Augustin, 12 years ago

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:16 by Anssi Kääriäinen, 12 years ago

Resolution: wontfix
Status: assignedclosed

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.

Note: See TracTickets for help on using tickets.
Back to Top