Opened 3 years ago

Closed 3 years ago

#32653 closed Bug (fixed)

Quoting names in the Oracle backend is not consistent with the db_table generation.

Reported by: Javier Buzzi Owned by: Mariusz Felisiak
Component: Database layer (models, ORM) Version: 2.2
Severity: Normal Keywords: oracle
Cc: Adam Johnson Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Original discussion can be found here: https://groups.google.com/g/django-developers/c/f3Z0AchkdyU/m/q0puWHYOCgAJ

I have a model with a really long name think ThisIsAReallyLongModelNameThatIsAlsoVeryOld. To complicate things further I have a Oracle instance with archival data designated with connection name old and a new postgres instance with the connection name default.

Issue i've found is that if i try and call the model ThisIsAReallyLongModelNameThatIsAlsoVeryOld.objects.using('old').count() i get an error saying that the table APP_THISISAREALLYLONGMODEL5300 does not exist, when it should be using APP_THISISAREALLYLONGMODEL5BD6 instead. When postgres is used it works as expected: ThisIsAReallyLongModelNameThatIsAlsoVeryOld.objects.using('default').count()

This is because the default connection is used when the model is instantiated, and then its used from that moment on.

https://github.com/django/django/blob/stable/3.2.x/django/db/models/options.py#L207

This is an issue that seems to go back to the beginning of Django.

Attachments (1)

test-32653.diff (1.2 KB ) - added by Mariusz Felisiak 3 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 by Simon Charette, 3 years ago

This is kind of have some overlap with the solution proposed by Anssi for #6148.

If db_table was a compilable object (it follows the as_sql(connection, compile) protocol) then the default implementation could be

class Table:
    def __init__(self, name):
        self.name = name

    def __str__(self):
        returns self.name

    def as_sql(self, connection, compiler):
        return connection.quote_name(
            truncate_name(name, connection.ops.max_name_length())
        )

And be used by Options.contribute_to_class as

db_table = self.db_table
if not db_table:
    db_table = "%s_%s" % (self.app_label, self.model_name)
self.db_table = Table(db_table)

comment:2 by Adam Johnson, 3 years ago

Cc: Adam Johnson added

comment:3 by Javier Buzzi, 3 years ago

@Simon YES! What you pasted is clear, concise, it's amazing. That looks like it should work. Whats the hold up?

comment:4 by Mariusz Felisiak, 3 years ago

Keywords: oracle added
Summary: Issue with multiple database backends sharing a model with a very long nameQuoting names in the Oracle backend is not consistent with the db_table generation.
Triage Stage: UnreviewedAccepted

I agree with Shai's comment. It's related with quoting names in the Oracle backend and can be fixed by:

diff --git a/django/db/backends/oracle/operations.py b/django/db/backends/oracle/operations.py
index ae6bd432fb..d48c8dd868 100644
--- a/django/db/backends/oracle/operations.py
+++ b/django/db/backends/oracle/operations.py
@@ -334,7 +334,7 @@ END;
         # always defaults to uppercase.
         # We simplify things by making Oracle identifiers always uppercase.
         if not name.startswith('"') and not name.endswith('"'):
-            name = '"%s"' % truncate_name(name.upper(), self.max_name_length())
+            name = '"%s"' % truncate_name(name, self.max_name_length())
         # Oracle puts the query text into a (query % args) construct, so % signs
         # in names need to be escaped. The '%%' will be collapsed back to '%' at
         # that stage so we aren't really making the name longer here.

by Mariusz Felisiak, 3 years ago

Attachment: test-32653.diff added

comment:5 by Javier Buzzi, 3 years ago

@Mariusz the issue is see with your solution is that it will work from my current case Postgres as default, Oracle as old. Where Postgres wont change the db_table as long as its under 63 chars??? But if we have the inverse, Oracle as default and Postgres as new; it will break Postgres since the db_table would be something like backends_verylongmodelname1234 and Postgres would want the whole string backends_verylongmodelnamezzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz. This is because the default connection is used to setup the table names at startup https://github.com/django/django/blob/stable/3.2.x/django/db/models/options.py#L207 . The solution @Simon suggested would work on both cases.

Ideally, the line self.db_table = truncate_name(self.db_table, connection.ops.max_name_length()) would be removed, and the aliases that is done in the SQLCompiler be moved to the individual backend, and that way you can cache the table names for later use.

Last edited 3 years ago by Javier Buzzi (previous) (diff)

comment:6 by Mariusz Felisiak, 3 years ago

Personally I would recommend specifying db_table for such a complicated setup with different vendors. Database backends (even builtin) will never be fully swappable. quote_name() on Oracle should be fixed regardless of whether we decide to move forward with Simon's proposition or not.

comment:7 by Mariusz Felisiak, 3 years ago

Ideally, the line self.db_table = truncate_name(self.db_table, connection.ops.max_name_length()) would be removed, and the aliases that is done in the SQLCompiler be moved to the individual backend, and that way you can cache the table names for later use.

This is tracked in #13528.

comment:8 by Mariusz Felisiak, 3 years ago

Has patch: set
Owner: changed from nobody to Mariusz Felisiak
Status: newassigned

comment:9 by GitHub <noreply@…>, 3 years ago

Resolution: fixed
Status: assignedclosed

In 1f643c2:

Fixed #32653 -- Made quoting names in the Oracle backend consistent with db_table.

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