Code

Opened 7 years ago

Closed 6 years ago

#5295 closed (fixed)

Inner Join and order_by bug

Reported by: MarioGonzalez <gonzalemario @…> Owned by: anonymous
Component: Database layer (models, ORM) Version: master
Severity: Keywords: qs-rf-fixed
Cc: gonzalemario@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: UI/UX:

Description

I found a problem using an inner join; these are some of my classes:

class Egreso(models.Model):
	id_egreso = models.AutoField(primary_key=True, db_index=True, db_column="id_egreso")
	id_usuario = models.ForeignKey(Usuario, db_index=True, db_column="id_usuario")	
	fecha_ingreso = models.DateTimeField(auto_now=True)

class Proveedor_combustible(models.Model):
	nombre = models.CharField(maxlength=200, unique=True)

	def __unicode__(self):
		return self.nombre

class Combustible(models.Model):
	id_egreso = models.ForeignKey(Egreso, db_index=True, db_column="id_egreso")
	numero_factura = models.PositiveIntegerField(null=True)
	proveedor = models.ForeignKey(Proveedor_combustible, db_column="proveedor")
	precio = models.CharField(maxlength=50)

	def __unicode__(self):
		return self.numero_factura

	class Meta:
		unique_together = ('numero_factura', 'proveedor'),

I want to execute a inner join and ordering the results using the right table, I always just did raw queries but now I tried to understand better the django api. This is the query I executed:

Combustible.objects.filter(id_egreso__id_usuario=id_usuario).order_by('gestion_combustible.fecha_ingreso')


However it fails because Django use an alias for the inner join; here is the SQL query that Django shows:

SELECT "gestion_combustible"."id","gestion_combustible"."id_egreso","gestion_combustible"."numero_factura","gestion_combustible"."proveedor","gestion_combustible"."precio"
FROM "gestion_combustible"
   INNER JOIN "gestion_egreso" AS "gestion_combustible__id_egreso"
   ON "gestion_combustible"."id_egreso" = "gestion_combustible__id_egreso"."id_egreso"
WHERE ("gestion_combustible__id_egreso"."id_usuario" = '1')
ORDER BY "gestion_egreso"."fecha_ingreso" ASC

As you can see the 2nd table is called gestion_egreso but the alias is called gestion_combustible_id_egreso so the ORDER BY should be done using the alias name not the field name. So far, the field name is used so it fails. The "solution": if you're using inners joins use the alias name in the order by.

But in my opinion, this must be done in the background.

Attachments (1)

joins_order_by.diff (781 bytes) - added by Peter Klein <petkle@…> 7 years ago.
accepts db_name as alias

Download all attachments as: .zip

Change History (6)

Changed 7 years ago by Peter Klein <petkle@…>

accepts db_name as alias

comment:1 Changed 7 years ago by Peter Klein <petkle@…>

  • Component changed from Template system to Database wrapper
  • Has patch set
  • Needs documentation unset
  • Needs tests set
  • Owner changed from nobody to anonymous
  • Patch needs improvement unset
  • Status changed from new to assigned
  • Triage Stage changed from Unreviewed to Design decision needed

To set ordering in this case you need to know the alias the query set manager for inner joins is using. Which is quite difficult to guess. One solution would be to explain this behaviour in the docs. A much more elegant solution would be to automatically look for a matching field when creating the ordering string. Then Mario Gonzeles and all of us would be able to use the table name to define the field. The attachment includes a small patch which should fix that.

comment:2 Changed 7 years ago by Peter Klein <petkle@…>

  • Keywords qs-rf added

comment:3 Changed 7 years ago by mtredinnick

  • Keywords qs-rf-fixed added; qs-rf removed
  • Triage Stage changed from Design decision needed to Accepted

The changes on the queryset-refactor branch for #2076 have fixed this as well. The new order_by() syntax automatically avoids the problems associated with aliasing. This ticket will be closed when the branch is merged into trunk.

comment:4 Changed 7 years ago by MarioGonzalez <gonzalemario @…>

Great! thanks Malcom

comment:5 Changed 6 years ago by mtredinnick

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [7477]) Merged the queryset-refactor branch into trunk.

This is a big internal change, but mostly backwards compatible with existing
code. Also adds a couple of new features.

Fixed #245, #1050, #1656, #1801, #2076, #2091, #2150, #2253, #2306, #2400, #2430, #2482, #2496, #2676, #2737, #2874, #2902, #2939, #3037, #3141, #3288, #3440, #3592, #3739, #4088, #4260, #4289, #4306, #4358, #4464, #4510, #4858, #5012, #5020, #5261, #5295, #5321, #5324, #5325, #5555, #5707, #5796, #5817, #5987, #6018, #6074, #6088, #6154, #6177, #6180, #6203, #6658

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.