feincms / django-tree-queries

Adjacency-list trees for Django using recursive common table expressions. Supports PostgreSQL, sqlite, MySQL and MariaDB.
https://django-tree-queries.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
432 stars 27 forks source link

quote order_by field in SQL #52

Closed taobojlen closed 1 year ago

taobojlen commented 1 year ago

Fix #51

This allows using an ordering field called order. Because order is a reserved keyword in SQL, this would cause errors previously.

I first reproduced the issue by changing the order field in the test Model. If you'd rather I create a new test model for this, let me know!

I only tested this with postgres locally, so I'm trusting your test suite for other databases. If I can do more to test these, again, please ask :)

matthiask commented 1 year ago

Thanks! Changing existing models is fine. I wonder if we shouldn't use connection.ops.quote_name but let's see what the tests say :)

taobojlen commented 1 year ago

looks like quote_name is the right approach! i'll give that a go.

i'm not sure what's going on with the 3.8 and 3.9 tests, though -- is that something you've seen before? if not i'll dig deeper!

matthiask commented 1 year ago

The next Django version will drop support for 3.8/3.9. Maybe you could push again so that we can re-check if it works now?

taobojlen commented 1 year ago

@matthiask yes! done, re-pushed :)

matthiask commented 1 year ago

Great, thanks!