Hello.

I found nice bug in your module when database has table prefixes. It crashed my site :)
I fixed bug and made patch. Please review it.

Comments

douggreen’s picture

Thanks! Is the ensure_table really needed? I thought that all views were based on nodes.

neochief’s picture

Yes it is required, because query builds like "...select * from prefix_node as node..." (or something like that) and your code in WHERE will be "prefix_node.created" , but not "node.created" as it should be

douggreen’s picture

StatusFileSize
new1.03 KB

So what your saying is {node}.created isn't expanded with the database prefix. If that's the case, then I think the solution is to expand it here rather than including the node table again.

Does my attached patch solve your problem?

neochief’s picture

Ive tried, but, nope, it still wrong.

here is the full buggy query with your patch

SELECT count( DISTINCT (
node.nid
) ) 
FROM m2b_node node
LEFT JOIN m2b_votingapi_cache votingapi_cache_vote_points_sum ON node.nid = votingapi_cache_vote_points_sum.content_id
AND votingapi_cache_vote_points_sum.content_type = 'node'
AND votingapi_cache_vote_points_sum.value_type = 'points'
AND votingapi_cache_vote_points_sum.tag = 'vote'
AND votingapi_cache_vote_points_sum.function = 'sum'
LEFT JOIN m2b_votingapi_cache votingapi_cache_vote_points_average ON node.nid = votingapi_cache_vote_points_average.content_id
AND votingapi_cache_vote_points_average.content_type = 'node'
AND votingapi_cache_vote_points_average.value_type = 'points'
AND votingapi_cache_vote_points_average.tag = 'vote'
AND votingapi_cache_vote_points_average.function = 'average'
WHERE (
(
node.type NOT 
IN (
'buzz', 'event', 'group'
)
OR node.type IS NULL 
)
)
AND (
votingapi_cache_vote_points_sum.value >0
)
AND (
m2b_node.created >1203563825
)
#1109 - Unknown table 'm2b_node' in where clause

as you see, your module puts "m2b_node.created", but it must put "node.created" to be fine

douggreen’s picture

StatusFileSize
new1020 bytes

Hmm, then it's possible that this is the patch needed.

neochief’s picture

Maybe you are right, but I saw other modules putting ensure_table() in this situation anyway. Actually I do not know what it's doing :) It's just works for me. And without it too.

douggreen’s picture

I'm not completely sure what you just said. Does the patch in #5 solve your problem? If so, then I'll commit it to th e project.

neochief’s picture

Yes, it solved the problem.

douggreen’s picture

Status: Needs review » Fixed

Thanks, committed

Anonymous’s picture

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.