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

Fixed a bug related to django4 foreign key lookups #647

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

HardlyForeal
Copy link

@HardlyForeal HardlyForeal commented Nov 19, 2022

Previous versions of django ran a foreign key lookup with a const (1) parameter in the column list. Django 4 changed that to a parameterized value and pass the parameter value 1. I have created a new token type to hold the aliased paramaterized value. SQLConstParameterizedIdentifier is subclassed from SQLConstIdentifier, and the original functionality is broken out into SQLConstIntIdentifier. This should allow anything that was using isinstanceof for SQLConstIdentifier to still work and allows expansion to other non-int constants.
A test case has been added for the django 4 select statement.
Closes #630
Closes #634

Previous versions of django ran a foreign key lookup with a const (1)
parameter in the column list.  Django 4 changed that to a parameterized
value and pass the parameter value 1.  I have created a new token type to
hold the aliased paramaterized value.  SQLConstParameterizedIdentifier
is subclassed from SQLConstIdentifier, and the original functionality is
broken out into SQLConstIntIdentifier.  This should allow anything that
was using isinstanceof for SQLConstIdentifier to still work and allows
expansion to other non-int constants.
A test case has been added for the django 4 select statement.
@HardlyForeal
Copy link
Author

Issue #630 was closed by the poster with the comment that they "fixed" it by reverting back to a prior django version. Other postings I've seen indicate that djongo is supposed to support the latest django version, so that issue should really still be open, and would be fixed by this pr. As far as I can tell, this is an issue with every foreign key relationship in the latest django due to changes in the sql it generates.

@ghpkishore
Copy link

@HardlyForeal Thanks! I had faced the same bug and used your commit files to update my local djongo files. Works like a charm now.

@pekosoG
Copy link

pekosoG commented Dec 23, 2022

Im facing exactly the same issue, @ghpkishore would you please share how did you manage to get it working? Did you use a monkey-patching in those methods?

@ghpkishore
Copy link

ghpkishore commented Dec 23, 2022

What ever files are present in this commit, I updated the files in my system with those. That worked. You can think of it as monkey-patching, but I changed the source code in the downloaded repo itself.

@deviantfero
Copy link

Is this going to be merged soon?

@Pansuria-Shrey
Copy link

Will it get merge anytime soon?

@phoenixsite
Copy link

Still working perfectly in Django 4.2.3 and the latest uploaded version of djongo. I had the exists() issue and also the validation of duplicate User usernames in the DB wasn't working. Thank you so much!

djongo/sql2mongo/sql_tokens.py Outdated Show resolved Hide resolved
djongo/sql2mongo/sql_tokens.py Outdated Show resolved Hide resolved
djongo/sql2mongo/sql_tokens.py Outdated Show resolved Hide resolved
phoenixsite added a commit to phoenixsite/djongo that referenced this pull request Sep 3, 2023
Added the changes included in pull request [doableware#647](doableware#647). Also it is added the ExtractFunc to extract a part from a date (inspired in MySQL syntax but slightly different). Finally, the F function was not working correctly when an incremenent is required.
@ELDiablO59152
Copy link

Thank you very much ! Please merge this fix

Copy link

@ELDiablO59152 ELDiablO59152 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please merge @doableware

@pimuzzo
Copy link

pimuzzo commented Jun 6, 2024

@nesdis please can you take a look on this?

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