I need to use earth_distance_sql() in a query that contains named tables and joins to the location table twice (one is in a db_rewrite_sql call from a different module).

The SQL emitted by the function assumes that the fields latitude and longitude will be unnamed. This patch allows for an optional table name to inject into the SQL and does so in a fully backward compatible way.

CommentFileSizeAuthor
#4 location_search_dist_sql_0.patch1.67 KBscott.mclewin
location_search_dist_sql.patch1.17 KBscott.mclewin

Comments

scott.mclewin’s picture

scott.mclewin commented

I remain interested in having this patch comitted. What additional information do you need from me?

ankur’s picture

ankur commented
Status: Reviewed & tested by the community » Needs work

Hey Scott,

Are you trying to pass in a full table name, or a table alias? If an alias, then there is no need for the brackets. Also, we should write the patch in a way where the '.' between the table name (or alias) and the name of the column ('latitude' or 'longitude' in this case) is supplied by the code in this function rather than being passed as part of the tbl_prefix parameter.

Finally, instead calling the optional parameter $tbl_prefix, we should call it $table_alias if the parameter is intended to be an alias (which I think is the way to go) or just $table_name if the parameter is intended to be the table name.

W

scott.mclewin’s picture

scott.mclewin commented

@ankur,

It is intended to be a table alias and I agree the name should be changed. I'll do that.

I'll also change the code to add the "." within the function.

As for the brackets {$tbl_prefix}, to be clear those are not appearing in the output string. While not I cannot be 100% sure from your comment, it seems you were reading those as literals being included in the output. They are not. It's a php nomenclature to allow a variable to be directly against another, which I did to allow the default value of $tbl_prefix ("") to do nothing while allowing an alias to be correctly incorporated. My apologies if I am needlessly going over something you already knew.

Scott

scott.mclewin’s picture

scott.mclewin commented
Status: Needs work » Reviewed & tested by the community
StatusFileSize
new location_search_dist_sql_0.patch1.67 KB

Here is the patch rolled again with the changes you requested.

ankur’s picture

ankur commented
Status: Reviewed & tested by the community » Fixed

Applied to DRUPAL-4-7 and DRUPAL-5.
Thanks for the patch.

scott.mclewin’s picture

scott.mclewin commented

No problem. Thanks for committing it and the review.

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)