Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error when adding to ManyToManyField #102

Open
ArthurNRL opened this issue Jun 13, 2022 · 5 comments · May be fixed by #103
Open

Error when adding to ManyToManyField #102

ArthurNRL opened this issue Jun 13, 2022 · 5 comments · May be fixed by #103

Comments

@ArthurNRL
Copy link

Subject: ProgrammingError when adding to ManyToManyField

Problem

  • Attempting to add to a many to many relationship results on a not supported query on Redshift (ON CONFLICT)

Procedure to reproduce the problem

class CustomerUserGroup(AbstractBaseModel):
    id = models.UUIDField(primary_key=True, default=uuid4, editable=False, unique=True, db_column='customerusergroup_id')


class OwnerGroup(AbstractBaseModel):
    id = models.UUIDField(primary_key=True, default=uuid4, editable=False, unique=True, db_column='ownergroup_id')
    customer_user_group = models.ManyToManyField(CustomerUserGroup)


customer_group = CustomerUserGroup.objects.all()[0]
OwnerGroup.objects.all()[0].customer_user_group.add(group)

Same thing happens when trying to add from the admin site

Error logs / results

Traceback (most recent call last):
  File "C:\~\venv\lib\site-packages\django\db\backends\utils.py", line 89, in _execute
    return self.cursor.execute(sql, params)
psycopg2.errors.SyntaxError: syntax error at or near "ON"
LINE 1: ...5c74ff2ce84', 'cf6d2a7cc06846e488905f8ed8844822') ON CONFLIC...
                                                             ^
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
  File "C:\~\AppData\Local\Programs\Python\Python310\lib\threading.py", line 946, in run
    self._target(*self._args, **self._kwargs)
  File "C:\~\venv\lib\site-packages\django\utils\autoreload.py", line 64, in wrapper
    fn(*args, **kwargs)
  File "C:\~\venv\lib\site-packages\django\core\management\commands\runserver.py", line 125, in inner_run
    autoreload.raise_last_exception()
  File "C:\~\venv\lib\site-packages\django\utils\autoreload.py", line 87, in raise_last_exception
    raise _exception[1]
  File "C:\~\venv\lib\site-packages\django\core\management\__init__.py", line 398, in execute
    autoreload.check_errors(django.setup)()
  File "C:\~\venv\lib\site-packages\django\utils\autoreload.py", line 64, in wrapper
    fn(*args, **kwargs)
  File "C:\~\venv\lib\site-packages\django\__init__.py", line 24, in setup
    apps.populate(settings.INSTALLED_APPS)
  File "C:\~\venv\lib\site-packages\django\apps\registry.py", line 124, in populate
    app_config.ready()
  File "C:\~\venv\lib\site-packages\django\contrib\admin\apps.py", line 27, in ready
    self.module.autodiscover()
  File "C:\~\venv\lib\site-packages\django\contrib\admin\__init__.py", line 50, in autodiscover
    autodiscover_modules("admin", register_to=site)
  File "C:\~\venv\lib\site-packages\django\utils\module_loading.py", line 58, in autodiscover_modules
    import_module("%s.%s" % (app_config.name, module_to_search))
  File "C:\~\AppData\Local\Programs\Python\Python310\lib\importlib\__init__.py", line 126, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1050, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1027, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1006, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 688, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 883, in exec_module
  File "<frozen importlib._bootstrap>", line 241, in _call_with_frames_removed
  File "C:\~\statement\admin\__init__.py", line 3, in <module>
    from .owner_group_admin import CustomOwnerGroupAdmin
  File "C:\~\statement\admin\owner_group_admin.py", line 26, in <module>
    OwnerGroup.objects.all()[0].customer_user_group.add(group)
  File "C:\~\venv\lib\site-packages\django\db\models\fields\related_descriptors.py", line 1048, in add
    self._add_items(
  File "C:\~\venv\lib\site-packages\django\db\models\fields\related_descriptors.py", line 1269, in _add_items
    self.through._default_manager.using(db).bulk_create(
  File "C:\~\venv\lib\site-packages\django\db\models\query.py", line 579, in bulk_create
    returned_columns = self._batched_insert(
  File "C:\~\venv\lib\site-packages\django\db\models\query.py", line 1467, in _batched_insert
    self._insert(
  File "C:\~\venv\lib\site-packages\django\db\models\query.py", line 1434, in _insert
    return query.get_compiler(using=using).execute_sql(returning_fields)
  File "C:\~\venv\lib\site-packages\django\db\models\sql\compiler.py", line 1621, in execute_sql
    cursor.execute(sql, params)
  File "C:\~\venv\lib\site-packages\django\db\backends\utils.py", line 103, in execute
    return super().execute(sql, params)
  File "C:\~\venv\lib\site-packages\django\db\backends\utils.py", line 67, in execute
    return self._execute_with_wrappers(
  File "C:\~\venv\lib\site-packages\django\db\backends\utils.py", line 80, in _execute_with_wrappers
    return executor(sql, params, many, context)
  File "C:\~\venv\lib\site-packages\django\db\backends\utils.py", line 84, in _execute
    with self.db.wrap_database_errors:
  File "C:\~\venv\lib\site-packages\django\db\utils.py", line 91, in __exit__
    raise dj_exc_value.with_traceback(traceback) from exc_value
  File "C:\~\venv\lib\site-packages\django\db\backends\utils.py", line 89, in _execute
    return self.cursor.execute(sql, params)
django.db.utils.ProgrammingError: syntax error at or near "ON"
LINE 1: ...5c74ff2ce84', 'cf6d2a7cc06846e488905f8ed8844822') ON CONFLIC...

Expected results

New record(s) get added to the database

Environment info

  • OS: Windows 11
  • Python version: 3.10.2
  • Django version: 4.0.4
  • Django-Redshift-Backend version: 3.0.0
@ArthurNRL
Copy link
Author

The full query:
INSERT INTO "statement_ownergroup_customer_user_group" ("ownergroup_id", "customerusergroup_id") VALUES ('2df4b23614514f93847b45c74ff2ce84', 'cf6d2a7cc06846e488905f8ed8844822') ON CONFLICT DO NOTHING

@ArthurNRL
Copy link
Author

Managed to fix it doing, although I'm not database savvy enough to know if this is safe or not

from django_redshift_backend.base import DatabaseOperations as RSDatabaseOperations, DatabaseWrapper as RSDatabaseWrapper


class DatabaseOperations(RSDatabaseOperations):

    def ignore_conflicts_suffix_sql(self, ignore_conflicts=None):
        return ''


class DatabaseWrapper(RSDatabaseWrapper):

    def __init__(self, *args, **kwargs):
        super(DatabaseWrapper, self).__init__(*args, **kwargs)
        self.ops = DatabaseOperations(self)

@shimizukawa
Copy link
Member

Thanks for yoru reporting and inspection!

Upon further investigation, it appears that INSERT ON CONFLICT was introduced in Postgres 9.5; https://www.postgresql.org/docs/9.5/sql-insert.html .
It also seems to be supported by the postgres backend in Django 2.2; django/django@f1fbef6 .

Redshift is based on Postgres 8.0.2 and does not support INSERT ON CONFLICT.
Therefore, INSERT ON CONFLICT must be disabled in some way.

I think your patch is a good solution because other backends that do not support ON CONFLICT simply return an empty string from ignore_conflicts_suffix_sql.
The only other thing I would do is to set DatabaseFeatures.supports_ignore_conflicts = False .

Since I don't have time to fix it now, I will prepare a UnitTest to reproduce the problem, fix it, and confirm that it works in the next few weeks. If you could prepare a PR for me, that would be great ;)

@ArthurNRL
Copy link
Author

I can do that for sure!

@shimizukawa
Copy link
Member

I would like to include this request/PR on #167.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants