Solución elegante (código en el controlador o en la vista)

Tengo una página en la que muestro información sobre cada tipo de
mensajes, y los últimos mensajes publicados de ese tipo. La primera
aproximación sería cargar en el controlador los tipos, y en la vista
hacer algo así:

<% @tipos.each do |tipo| %>
— Mostrar los datos del tipo —
<% @mensajes = Mensajes.find(:all, :order => “created_at DESC”, :limit
=> 3, :conditions => “tipo_id = #{tipo}” ) %>
— Mostrar los últimos mensajes del tipo —
<% end %>

Esto funciona bien, pero es feo porque mete en la vista una carga de
datos que debería estar en el controlador. ¿Cómo podría resolver esto de
una forma más elegante?

s2 y gracias

2008/2/28 Fernando C. [email protected]:

Esto funciona bien, pero es feo porque mete en la vista una carga de
datos que debería estar en el controlador. ¿Cómo podría resolver esto de
una forma más elegante?

Sin más información, creo que la forma correcta a nivel MVC
sería así:
Modelos:

class Tipo < AR::Base
has_many :mensajes
end

class Mensaje < AR::Base
belongs_to :tipo
end

Controlador:

@tipos = Tipo.find(:all)

opcionalmente, Tipo.find(:all, :include => :mensajes)

Vista:

<% @tipos.each do |tipo| %>
– Datos de tipo –
<% tipo.mensajes.each do |mensaje| %>
– Datos de mensaje –
<% end %>
<% end %>


Sergio Gil Pérez de la Manga
e-mail > [email protected]
blog > http://www.lacoctelera.com/porras

Perdón, se me ha olvidado un detalle: la tabla de mensajes es enorme
(varios cientos de miles), y sólo tengo que mostrar los 3 últimos. Un
acceso como el que proponéis (que obviamente es la solución correcta)
tendría un coste muy alto y funcionaría mal, por lo que de entrada no es
viable.

Lo que quizá sí sería viable sería definir una nueva relación, un poco
más elaborada: que en vez de

Tipo.rb
has_many :mensajes

fuera algo así:
has_many :ultimos_mensajes, :order => ‘created_at desc’, :limit => 3

Y declarar una clase ultimos_mensajes que herede de mensajes… ¿qué os
parece eso?

Hola Fernando

Al ver tipo_id, supongo que tendras una relación por el medio, con lo
que cada tipo ya tiene sus mensajes. para optimizar un poco en el
controlador podría meter algo del estilo:

@tipos = Tipo.find(:all, :include => :mensajes) #carga tambien los
mensajes

Para mostrar por pantalla los mensajes puedes hacer un parcial para los
mensajes
y en la vista te quedaría algo así:

<% for tipo in @tipos %>
<%= render :partial => “mensajes”, :locals => {:mesajes =>
tipo.mensajes} %>
<% end -%>

Esta forma la suelo usar bastante y me funicona bastante bien.

Un saludo.

2008/2/28 Fernando C. [email protected]:

Esto funciona bien, pero es feo porque mete en la vista una carga de
datos que debería estar en el controlador. ¿Cómo podría resolver esto de
una forma más elegante?

s2 y gracias

Hay varios problemillas con tu código, obviamente el primero ya lo
comentas tú: pones en la vista lógica de la aplicación, lo que no es
nada recomendable. Pero supongamos que no tenemos problemas con eso
(por ahora).

Para obtener los mensajes de un tipo utilizas :conditions => “tipo_id
= #{tipo}” que tiene dos problemas. En tú caso un problema de
inyección de código es difícil, pero sería recomendable que en esas
cadenas de conditions, permitieras a Rails que te protegiera
utilizando :conditions => { :tipo_id => tipo.id }.

Pero además no necesitas construir tú mismo las condiciones SQL cuando
tienes una relación has_many/belongs_to entre los modelos Tipo y
Mensaje, desde un objeto de clase Tipo puedes utilizar el método
mensajes y desde un objeto de clase Mensaje el método tipo, incluso
con finders y demás cosas que te proporciona Rails. De forma que tu
línea para recupera mensajes sería:

@mensajes = tipo.mensajes.find(:limit => 3, :order => ‘created_at DESC’)

Mi recomendación sería seguir eliminando lógica de la vista y crear un
nuevo método en mensajes llamado ultimos que precisamente hiciera esa
búsqueda:

class Mensaje
def self.ultimos(limit = 3) # Por defecto 3, pero se puede modificar
al llamar al método
find(:limit => limit, :order => ‘created_at DESC’)
end
end

Un método que mágicamente Rails te permite utilizar “a través” de una
asociación:
@mensajes = tipo.mensajes.ultimos

El último problema de tu código es el “temido” problema de las n+1
consultas: tú código hace una primera consulta para obtener todos los
tipos, y luego, para cada uno de ellos (n) hace otra consulta para
obtener los tres últimos mensajes. Normalmente ese problema se
resuelve con lo que se denomina “eager_loading”, utilizando la
opción:include de Rails o algo similar, pero no consigo ver con tus
condiciones sobre los mensajes como se puede obtener los últimos tres
de cada tipo directamente desde la base de datos. Quizá alguien con
más experiencia en SQL puede ver una
solución.
Suerte.

Sergio Gil Pérez de la Manga wrote:

No har�a falta crear otra clase, bastar�a con especificar la clase en
la relaci�n, algo
as�:
class Tipo < AR::Base
has_many :mensajes
has_many :ultimos_mensajes, :class_name => ‘Mensaje’, :order =>
‘created_at desc’, :limit => 3
end

Si no me equivoco… (estoy escribiendo el c�digo directamente
aqu�)

Sergio Gil P�rez de la Manga
e-mail > [email protected]
blog > http://www.lacoctelera.com/porras

Sergio, muchas gracias, tu solución es cojonuda… pero me temo que en
términos de rendimiento es un desastre. Aunque no lo había mencionado
antes, por no salirnos del problema, desde mensajes se linka también con
la tabla usuarios (el autor del mensaje). Y al probar esto:

@tipos = Tipo.find(:all, :order => ‘position’, :include =>
{:ultimos_contenidos_3 => :usuario})

se genera una consulta tan brutal que mysql lleva un cuarto de hora y
aún no me ha devuelto el resultado.

No puedo pasar por abstracciones de rails porque no generan consultas
óptimas, y con estas tablas tan grandes el que sea o no óptima es
crítico. Por lo tanto, no tengo más remedio que montar más o menos “a
mano” la consulta. Voy a probar a hacerlo con un método de la clase,
como sugiere Daniel, a ver qué resultado da… ya os contaré.

s2

2008/2/28 Fernando C. [email protected]:

has_many :mensajes

fuera algo así:
has_many :ultimos_mensajes, :order => ‘created_at desc’, :limit => 3

Y declarar una clase ultimos_mensajes que herede de mensajes… ¿qué os
parece eso?

No haría falta crear otra clase, bastaría con especificar la clase en
la relación, algo
así:
class Tipo < AR::Base
has_many :mensajes
has_many :ultimos_mensajes, :class_name => ‘Mensaje’, :order =>
‘created_at desc’, :limit => 3
end

Si no me equivoco… (estoy escribiendo el código directamente
aquí)

Sergio Gil Pérez de la Manga
e-mail > [email protected]
blog > http://www.lacoctelera.com/porras

Hola fernando, la solucion que te dio pablo me parece buena, solo
cambiar lo
siguiente:

<% for tipo in @tipos %>
<%= render :partial => “mensajes”, :locals => {:mesajes =>
tipo.mensajes}
%>
<% end -%>

por;

<% for tipo in @tipos %>
<%= render :partial => “mensaje”, :collection => tipo.mensajes[0…2]
%>
<% end -%>

Aqui hay varias cosas importantes, primero al usar collection tendrias
la
variable local mensaje disponible automaticamnete en el partial.

Ahora por el lado del SQL, veo un problema y es que quieras o no estas
cargando todos los mensajes asociados al tipo; ahora como el metodo
‘mensajes’ te devuelve un array entonces puedes jugar y seleccionar la
cantidad de elementos que necesites, aunque tal vez no estan ordenados
como
tu quieras, eso ya es cosa de que juegues un poco mas con los metodos
del
array.

Otra solucion tal vez un poco mas pragmatica seria usar una sentencia
SQL
pura y dura en tu controlador para obtener los mensajes, y entonces te
evitarias de liarte tanto en las vistas.

Saludos.

El día 28/02/08, Fernando C. [email protected]
escribió:

Fernando comprendo exactamente tu situacion, por eso me temia que la
solución que te di no iba a ser la mas óptima, ademas que no habia leido
tus
otros mensajes, bueno ya que necesitas bastante optimización de la
consulta,
tal vez este slide1 te pueda servir.

Saludos

El día 28/02/08, Fernando C. [email protected]
escribió:

Ruben Davila wrote:

Hola fernando, la solucion que te dio pablo me parece buena, solo
cambiar lo
siguiente:

<% for tipo in @tipos %>
<%= render :partial => “mensajes”, :locals => {:mesajes =>
tipo.mensajes}
%>
<% end -%>

por;

<% for tipo in @tipos %>
<%= render :partial => “mensaje”, :collection => tipo.mensajes[0…2]
%>
<% end -%>

Aqui hay varias cosas importantes, primero al usar collection tendrias
la
variable local mensaje disponible automaticamnete en el partial.

Ahora por el lado del SQL, veo un problema y es que quieras o no estas
cargando todos los mensajes asociados al tipo; ahora como el metodo
‘mensajes’ te devuelve un array entonces puedes jugar y seleccionar la
cantidad de elementos que necesites, aunque tal vez no estan ordenados
como
tu quieras, eso ya es cosa de que juegues un poco mas con los metodos
del
array.

Otra solucion tal vez un poco mas pragmatica seria usar una sentencia
SQL
pura y dura en tu controlador para obtener los mensajes, y entonces te
evitarias de liarte tanto en las vistas.

Saludos.

El día 28/02/08, Fernando C. [email protected]
escribió:

Yo no puedo estar cargando todos los mensajes, hablamos de más de
100.000!! Tengo que irme a SQL propia, pero el tema es que son n SQLs, y
a ver cómo meto un nº indeterminado de SQLs en el controlador… por eso
el primer intento ha sido meterla en la vista (que es una chapuza, pero
va como una bala).

s2

Daniel R. Troitiño wrote:

2008/2/28 Fernando C. [email protected]:

Esto funciona bien, pero es feo porque mete en la vista una carga de
datos que deber�a estar en el controlador. �C�mo podr�a resolver esto de
una forma m�s elegante?

s2 y gracias

Hay varios problemillas con tu c�digo, obviamente el primero ya lo
comentas t�: pones en la vista l�gica de la aplicaci�n, lo que no es
nada recomendable. Pero supongamos que no tenemos problemas con eso
(por ahora).

Para obtener los mensajes de un tipo utilizas :conditions => “tipo_id
= #{tipo}” que tiene dos problemas. En t� caso un problema de
inyecci�n de c�digo es dif�cil, pero ser�a recomendable que en esas
cadenas de conditions, permitieras a Rails que te protegiera
utilizando :conditions => { :tipo_id => tipo.id }.

Pero adem�s no necesitas construir t� mismo las condiciones SQL cuando
tienes una relaci�n has_many/belongs_to entre los modelos Tipo y
Mensaje, desde un objeto de clase Tipo puedes utilizar el m�todo
mensajes y desde un objeto de clase Mensaje el m�todo tipo, incluso
con finders y dem�s cosas que te proporciona Rails. De forma que tu
l�nea para recupera mensajes ser�a:

@mensajes = tipo.mensajes.find(:limit => 3, :order => ‘created_at DESC’)

Mi recomendaci�n ser�a seguir eliminando l�gica de la vista y crear un
nuevo m�todo en mensajes llamado ultimos que precisamente hiciera esa
b�squeda:

class Mensaje
def self.ultimos(limit = 3) # Por defecto 3, pero se puede modificar
al llamar al m�todo
find(:limit => limit, :order => ‘created_at DESC’)
end
end

Un m�todo que m�gicamente Rails te permite utilizar “a trav�s” de una
asociaci�n:
@mensajes = tipo.mensajes.ultimos

El �ltimo problema de tu c�digo es el “temido” problema de las n+1
consultas: t� c�digo hace una primera consulta para obtener todos los
tipos, y luego, para cada uno de ellos (n) hace otra consulta para
obtener los tres �ltimos mensajes. Normalmente ese problema se
resuelve con lo que se denomina “eager_loading”, utilizando la
opci�n:include de Rails o algo similar, pero no consigo ver con tus
condiciones sobre los mensajes como se puede obtener los �ltimos tres
de cada tipo directamente desde la base de datos. Quiz� alguien con
m�s experiencia en SQL puede ver una
soluci�n.
Suerte.

Daniel, gracias por todas las correcciones.

  • Efectivamente, aquí no hay posibilidad de inyección de código, los
    tipos los maneja el administrador.

  • Respecto a construir las condiciones SQL yo mismo, sí necesito
    hacerlo: cuestión de rendimiento crítico. Ir del tipo a los mensajes es
    lentísimo, mientras que ir directamente a los mensajes, filtrando por
    tipo, es instantáneo… incluso aunque sea n+1 (en este caso, n es
    pequeño).

  • Sin embargo, tu idea de mover el código a la clase es muy buena…
    sólo que por tema de eficiencia, sería en la clase Tipo y no el la
    Mensaje (si lo meto en mensaje, volvemos al mismo problema). Metiendo en
    Tipo.rb esto:

    def ultimos_contenidos(limit = 3) # Por defecto 3, pero se puede
    modificar al llamar al método
    Mensaje.find(:all, :limit => limit, :order => “contenidos.created_at
    DESC”, :conditions => “tipo_id = #{self.id} and publicado”)
    end

en la vista ya puedo hacer:

tipo.ultimos_contenidos.each do |msg|

Y va bien.

2008/2/28 Fernando C. [email protected]:

nuevo m�todo en mensajes llamado ultimos que precisamente hiciera esa
asociaci�n:
m�s experiencia en SQL puede ver una
tipo, es instantáneo… incluso aunque sea n+1 (en este caso, n es
end
Posted via http://www.ruby-forum.com/.


Ror-es mailing list
[email protected]
simplelogica.net

Cuando utilizas un método de clase (los que llevan self) a través de
una asociación Rails añade automáticamente la condición que tu
escribes para tipo_id (tendrías que añadir la de publicado al método
de clase, como antes no la ponías en mi solución no la puse). No creo
que el código SQL generado de ese modo y el código que se genera con
tu método sea muy diferente.

De todas formas, por lo que hablas, sobre la cantidad de datos y
demás, no me parecen tantos como para que MySQL tarde tanto en
cargarlos. ¿Tienes bien puestos los indices de las tablas?
Necesitarías al menos un índice para tipo_id y otro para created_at en
la tabla mensajes. Sin esos dos índices MySQL va a tener que leer cada
uno de los registros para encontrar los que quieres.

Si crees que es la base de datos la que no da la talla podrías
utilizar el plugin Query Analyzer
http://agilewebdevelopment.com/plugins/query_analyzer para comprobar
que tus queries SQL están utilizando los índices correctamente.

Suerte.

Perdonad por desviarme del tema, pero de donde puedo encontrar
informacion acerca de este tipo de relaciones (la de
“ultimos_mensajes”)?

class Tipo < AR::Base
has_many :mensajes
has_many :ultimos_mensajes, :class_name => ‘Mensaje’, :order =>
‘created_at desc’, :limit => 3

end

Siempre habia visto relaciones normales con otros objectos, pero nunca
en las que especificabas esos parametros.

Gracias y disculpas de nuevo por desviarme del tema principal.

Sergio Gil Pérez de la Manga
escribió:> 2008/2/28 Fernando C. [email protected]:

2008/2/29 Fernando C. [email protected]:

http://agilewebdevelopment.com/plugins/query_analyzer para comprobar

Me temo que esos SELECT count pueden tener un impacto dramático si
empiezan a lanzarse de forma repetida…

s2

En las sentencias “select count…” creo que los índices son
necesarios sobre las tres columnas a la vez, pero podría equivocarme.
Te recomiendo ejecutar esas sentencias “select count…” sobre tu base
de datos con “explain select count…” para ver si MySQL utiliza
correctamente los índices.

De cualquier manera, si MySQL utiliza bien los índices y aún te parece
que la eficiencia no es la adecuanda lo más recomendable
seríade-normalizar, es decir, incluir en la tabla “tipos” dos columnas
“temas_count” y “respuestas_count” que se actualizaran cuando se
crearan nuevos mensajes o se destruyeran los ya existentes.

Suerte.

2008/2/29 Javier Martínez [email protected]:

Siempre habia visto relaciones normales con otros objectos, pero nunca
en las que especificabas esos parametros.

Gracias y disculpas de nuevo por desviarme del tema principal.

Obviamente en la ayuda de Rails:
http://api.rubyonrails.org/classes/ActiveRecord/Associations/ClassMethods.html#M001103

Daniel R. Troitiño wrote:

De todas formas, por lo que hablas, sobre la cantidad de datos y
demás, no me parecen tantos como para que MySQL tarde tanto en
cargarlos. ¿Tienes bien puestos los indices de las tablas?
Necesitarías al menos un índice para tipo_id y otro para created_at en
la tabla mensajes. Sin esos dos índices MySQL va a tener que leer cada
uno de los registros para encontrar los que quieres.

Si crees que es la base de datos la que no da la talla podrías
utilizar el plugin Query Analyzer
http://agilewebdevelopment.com/plugins/query_analyzer para comprobar
que tus queries SQL están utilizando los índices correctamente.

Suerte.

Los índices están bien puestos: en tipo_id, en created_at y en
usuario_id. Pero con la consulta “estandar” de ruby, es un desastre de
rendimiento, mientras que al partirlo en n+1 ya va bien. ¿La causa? Pues
aunque sólo había puesto lo que me parecía más relevante, la consulta
sobre tipos lleva más cosas; no sé si será por eso el desastre de
algunas combinaciones. Esta es la verdadera consulta, completa:

@tipos = Tipo.find(:all, :order => 'tipos.position', :conditions => 

“clase_id = 1”,
:select => “tipos.id, tipos.nombre_completo,
url_tipo, t.url_clase, tipos.descripcion,
(SELECT count() FROM mensajes WHERE
tipo_id = tipos.id and publicado and inicial_id = 0) as temas,
(SELECT count(
) FROM mensajes WHERE
tipo_id = tipos.id and publicado and inicial_id <> 0) as respuestas”,
:joins => “join clase t on tipos.clase_id = t.id”)

Me temo que esos SELECT count pueden tener un impacto dramático si
empiezan a lanzarse de forma repetida…

s2

Daniel R. Troitiño wrote:

De cualquier manera, si MySQL utiliza bien los �ndices y a�n te parece
que la eficiencia no es la adecuanda lo m�s recomendable
ser�ade-normalizar, es decir, incluir en la tabla “tipos” dos columnas
“temas_count” y “respuestas_count” que se actualizaran cuando se
crearan nuevos mensajes o se destruyeran los ya existentes.

Suerte.

Por ahí voy a tirar… un poco de de-normalización, cacheando los datos
intermedios, y ya tengo un rendimiento óptimo… y eso sí, un foco de
problemas potenciales si meto la pata. Pero es lo que toca para este
caso, creo.

s2

El día 28/02/08, Fernando C. [email protected]
escribió:

Perdón, se me ha olvidado un detalle: la tabla de mensajes es enorme
(varios cientos de miles), y sólo tengo que mostrar los 3 últimos. Un
acceso como el que proponéis (que obviamente es la solución correcta)
tendría un coste muy alto y funcionaría mal, por lo que de entrada no es
viable.

En la Conferencia Rails 2007, Pablo Delgado expuso una charla super
interesante sobre escalabilidad:

http://video.google.es/videoplay?docid=-3750147543196804850
conferenciarails.org - contact with domain owner | Epik.com

Uno de los trucos para mostrar rápidamente los x últimos registros de
una
tabla enorme era crear un índice inverso, en plan:

reverse_created_at = 2030 - created_at

Así el MySQL los encuentra mucho más rápidamente, por lo visto.

On 03/03/2008, Fernando C. [email protected]
wrote:

intermedios, y ya tengo un rendimiento óptimo… y eso sí, un foco de
problemas potenciales si meto la pata. Pero es lo que toca para este
caso, creo.

Ya lo dijo Cal Henderson

http://www.google.es/search?hl=en&client=firefox-a&rls=org.mozilla%3Aen-US%3Aofficial&hs=FfD&q=normalized+data+is+for+sissies&btnG=Search


Manuel, que
piensa que eres una excelente persona y medra en torno a
http://simplelogica.net y/o http://simplelogica.net/logicola/
Recuerda comer mucha fruta y verdura.

El 1 de marzo de 2008 18:13, Jaime I.
[email protected]escribió:

reverse_created_at = 2030 - created_at

Así el MySQL los encuentra mucho más rápidamente, por lo visto.

Estoy probando esta técnica que expuso Pablo Delgado para mostrar
rápidamente los X últimos registros de las tablas… lo que él expone es
que
como MySQL tarda mucho en ordenar una tabla a la inversa, si por ejemplo
quieres sacar los últimos 10 posts de una tabla de un millón de posts,
es
crear un índice inverso respecto a la fecha de creación, en plan

reverse_created_at = 2030 - created_at

Bueno, lo primero con lo que me encuentro al llevarlo a la práctica es
que
no se puede restar así directamente, 2030 es un integer y created_at una
fecha así que lo pongo así:

class Post < ActiveRecord::Base

before_save :update_reverse

def update_reverse
self.reverse_created_at = ((‘2030/1/1’.to_datetime) -
(self.created_at.to_datetime))
self.reverse_updated_at = ((‘2030/1/1’.to_datetime) -
(self.updated_at.to_datetime))
end

end

Pero esto no me funciona, porque de la resta me devuelve un tipo de
datos
Rational que no se guarda en el MySQL…

Rational(676652623, 86400)

¿Qué estoy haciendo mal?