Skip to content

fix(django-cf): fix order_by not working properly - #202

Merged
ryanking13 merged 2 commits into
mainfrom
gyeongjae/django-cf-order
Aug 13, 2026
Merged

fix(django-cf): fix order_by not working properly#202
ryanking13 merged 2 commits into
mainfrom
gyeongjae/django-cf-order

Conversation

@ryanking13

Copy link
Copy Markdown
Contributor

Fixes a bug that the fetch order is always fixed for the d1, do backend.

Comment thread packages/django-cf/django_cf/db/base_engine.py
@ask-bonk

ask-bonk Bot commented Aug 13, 2026

Copy link
Copy Markdown

I'm Bonk, and I've done a quick review of your PR.

This PR replaces the destructive list.pop() approach in CFResult.fetchone/fetchall/fetchmany with a position-based cursor, fixing the ordering bug and preserving the underlying data.

  1. Missing _position initialization (high severity): self._position is referenced by all three fetch methods but is never set in __init__. Every call to fetchone(), fetchall(), or fetchmany() will raise AttributeError. Add self._position = 0 in CFResult.__init__.

One inline comment was submitted with this finding.

github run

@ryanking13
ryanking13 force-pushed the gyeongjae/django-cf-order branch from 194d687 to 9e92653 Compare August 13, 2026 06:38

@dom96 dom96 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Curious why the changes to packages/django-cf/django_cf/db/base_engine.py are needed, but otherwise looks good.

Comment thread packages/django-cf/django_cf/db/base_engine.py
@ryanking13
ryanking13 merged commit 3010b06 into main Aug 13, 2026
26 checks passed
@ryanking13
ryanking13 deleted the gyeongjae/django-cf-order branch August 13, 2026 15:44
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 this pull request may close these issues.

2 participants