databasejoin plugin - issue with additional sql

W

WolfgangA

New Member
I use the database join plugin with additional sql in the field "Joins where and/or order by statement (SQL)". Here my add. sql:
Code: 
JOIN wa_contenttype_to_category AS a
ON a.contenttype_id = 1
WHERE joomla_content.catid = a.category_id

This leads to the following sql created by fabrik, which is wrong:
Code: 
SELECT DISTINCT(`joomla_content`.`id`) AS value, `title` AS text 
FROM `joomla_content` AS `joomla_content` 
WHERE JOIN wa_contenttype_to_category AS a 
ON a.contenttype_id = 1 WHERE joomla_content.catid = a.category_id 
ORDER BY text ASC

The problem is that fabrik inserts a "WHERE" right at the beginning of the additional SQL. See "WHERE JOIN..."

Any hint how to solve this?

Thank you in advance.

P.S.
Fabrik - latest version 3.x from svn of 18/3/2013
DatabaseJoin element as of its xml: 3.0.7
Database Version 5.0.51a-24+lenny5
Database Collation utf8_general_ci
PHP Version 5.2.6-1+lenny16
Web Server Apache
WebServer to PHP Interface apache2handler
Joomla! Version Joomla! 2.5.9 Stable [ Ember ] 4-February-2013 14:00 GMT
 
Hmm - this seems to be a bigger one than i initially thought.

The current databasejoin plugin makes use of the joomla query object to prepare the custom sql.
(see protected function _buildQueryWhere around line 709)
It uses joomlas $query->where wich prepends "WHERE".

So basically the backend of fabrik is wrong saying "Joins where and/or order by statement (SQL)".

So far I see two options:
- do not use the joomla activerecord database object $query.
Build the entire query inside fabrik.
(This seems to be the easiest)

or

- go with the joomla $query object. Seperate the input and create 2 backend fields: one for JOIN(s) and one for WHERE. Maybe even a third one for "ORDER BY"...
(Though i am not quite sure how to handle multiple joins, bc the $query object prepends "JOIN" so the user would have to omnit the first but not further ones.
Or each join must have its own input - makes backend more complicated...)

It is really sad because the current implementation limits the power of this cool plugin a lot :(

Anyone having thoughts on this?

-------
P.S. The use-case is: Allow to define contenttypes based on native joomla categories and let the user assign articles to that specific related contenttype.

- Each contentype has its own table with properties.

- Each contentype use a map table (wa_contenttype_to_category) that has a one-to-many relationship to joomla categories.

- The article's contentype is then defined /based on the articles categorie.

So the databasejoin selects only articles that have categories within that map table.

All of this is build with fabrik tables.
 
WolfgangA said:
I use the database join plugin with additional sql in the field "Joins where and/or order by statement (SQL)". Here my add. sql:
Code: 
JOIN wa_contenttype_to_category AS a
ON a.contenttype_id = 1
WHERE joomla_content.catid = a.category_id
Click to expand...

This field in admin form is only for WHERE and/or ORDER BY statement.
Additional sql, if needed, can be used in concat label field.
See http://fabrikar.com/wiki/index.php/Database_join
 
Thank you for your reply. But sorry to say, the concat field cannot be used in this way. It wraps the code (as it says) in a CONCAT statement. Example from test:
Code: 
SELECT SQL_CALC_FOUND_ROWS DISTINCT `wa_content_switch`.`id` AS `wa_content_switch___id`, 
`wa_content_switch`.`id` AS `wa_content_switch___id_raw`, `wa_content_switch`.`article_id` AS `wa_content_switch___article_id_raw`, 
CONCAT(JOIN wa_contenttype_to_category AS a ON a.contenttype_id = 1) AS `wa_content_switch___article_id`,
 
Looking to the code, i think i can fix this. Would need to change:
Code: 
arround line 597
protected function buildQuery($data = array(), $incWhere = true, $opts = array())

and

Code: 
arround line 698
protected function buildQueryWhere($data = array(), $incWhere = true, $thisTableAlias = null, $opts = array(), $query = false)array())

The way it would work:
- "$incWhere" what have more a meaning of "Append custom sql to query". So any sql that can stay after a SELECT or JOIN would be allowed.

- if custom Sql is used, no automatic prepend of WHERE. (which is similar to the example of the current doc / wiki).

- a check would happen if the custom Sql has its own "ORDER BY". If not, then the default is appended.

This should not break compatibility to teh existing implentation.

If a developer reads this:
Given an acceptable quality of implementation, any chance this functionallity can go to the plugin permanently?
 
Why you need just a join statement inside databasejoin element that itself has a join functionality? Just interesting to know, perhaps I missed some potentially good feature :)
 
As i explained before - its used to automatically get only the articles which belong to a "cck" category. Quoing myself:

-------
P.S. The use-case is: Allow to define contenttypes based on native joomla categories and let the user assign articles to that specific related contenttype.

- Each contentype has its own table with properties.

- Each contentype use a map table (wa_contenttype_to_category) that has a one-to-many relationship to joomla categories.

- The article's contentype is then defined /based on the articles categorie.

So the databasejoin selects only articles that have categories within that map table.

All of this is build with fabrik tables.
Click to expand...
 
In principle, I'm not against adding YAFO (Yet Another Option) to the join element, to add an optional JOIN statement.

I'd rather not include it in the existing WHERE option, as I can already think of issues with trying to do that. I can see where you are going with your suggestion, but there are some gory details not obvious in that part of the code which would mean it would still break the query if the WHERE option started with a JOIN.

We already have a buildQueryJoin() method, which doesn't do anything in the main join element model, but is used in other elements that extend the join element class. And we already have code elsewhere that takes textual join strings and busts them up into Query Builder Speak.

So it wouldn't be that much work to just add YAFO, where you can specify:

JOIN string: LEFT JOIN foo on {thistable}.id = foo.parent_id, INNER JOIN bar on bar.something = foo.whatever

... and for us to add that to the query building, allowing you to then use foo and bar fields in your WHERE. And (I think) in the CONCAT label.

However, it's currently 5:20am, which is somewhat too late for me to be messing with join code.

So try bumping this in a day or so, and I'll have a go at it.

-- hugh
 
Thank you for taking the time reading this stuff. I would have gone the easy route forcing buildQueryWhere() to return a string and then add the string to the select part created in buildQuery().

Or - i would have created the entire sql as a string and handed over to an empty query object with setQuery() inside buildQuery().

But - most probaly your are right in that i do of cause not know all sideeffects. In fact I was just looking into the plugin code for references to buildQueryWhere() and thought i would not break things.

Nevertheless - Just a note: My post was i no way meant to get someone else to implement this. So thank you or your trouble.

Wolfgang
 
We always welcome others to contribute code, through pull requests in github, and appreciate anyone who takes the time to look at our code (which can be somewhat ... opaque, to say the least).

The problem is that things like the join code gets VERY involved, and has a lot of hidden gotchas that aren't immediately obvious. So in this case, it would probably take longer for us to review your changes and get you to fix issues we see with it, rather than just doing it myself.

I've had quite a few people after me about getting additional JOIN's to work in the join element. Although it was never designed to support it, the "WHERE" option used to allw it, just through happy accident. But since we rebuilt all that to use the query builder, the "happy accident" went away. So a number of folks are in the position of relying on an "undocumented feature" which has now gone away.

I'll go ahead and have a hack at it right now.

- hugh
 
Btw, thought you might find my conversation with Rob about this amusing:

[6:40:13 AM] Hugh Messenger: I've had half a dozen people tell me in the last few months that their "JOIN" statement in the WHERE clause filter no longer works.
[6:40:45 AM] Hugh Messenger: Which i didn't think it ever did, but apparently until we made some change or other, you could actually do "JOIN FOO on blah blah WHERE blah"
[6:41:24 AM] Hugh Messenger: I think it was prolly when we Query Builder-ified it.
[6:41:33 AM] Hugh Messenger: Anyway.
[6:42:01 AM] Hugh Messenger: As we already have a buildQueryJoin() method, which doesn't do anything except in (I think the Notes element.
[6:42:31 AM] Hugh Messenger: And code knocking around which pulls a textual join clause apart and turns it into Query Builder Speak.
[6:42:59 AM] Hugh Messenger: It would be fairly trivial to add an optional "JOIN clause" to the join element.
[6:43:26 AM | Edited 6:43:27 AM] Rob Clayburn: "simple" (tm)
[6:43:28 AM] Hugh Messenger: check for it in the existing buildQueryJoin() method, and Query Builderify it there.
[6:43:31 AM] Hugh Messenger: LOL!
[6:43:45 AM] Hugh Messenger: Yeah, but this time I've actually Check The Code <tm>
[6:43:55 AM] Rob Clayburn: but yeah cant see (cough cough) what could go wrong
[6:44:04 AM] Hugh Messenger: Exactly!!!!
[6:44:11 AM] Hugh Messenger: And really, How Hard Can It Be?
[6:44:49 AM] Rob Clayburn: 5 minute job for sure

-- hugh
 
OK, I've had a hack at it, I think it works. Should support all of the valid MySQL join types.

I've done basic testing, with two joins, a LEFT and a RIGHT OUTER, and using results from the those joins in the WHERE and CONCAT parts, seems to work OK.

So, for instance, with a JOIN of "LEFT JOIN #__users ON {thistable}.userid = #__users.id", I am then able to use #__users.email in my "CONCAT label".

NOTE - one thing I'm doing as yet is full placeholder replacement on the JOIN clause. So it currently only handle the standard J! #__, and our {thistable}

Should be In github.

Let me know how you get on.

-- hugh
 
Thank you very much for your work. Well i tried the new stuff after doing a _full_ github update.

i put this in the new join field:
Code: 
JOIN wa_contenttype_to_category AS a
ON a.contenttype_id = 1

and this in the where field:
Code: 
WHERE joomla_content.catid = a.category_id

and got the following error:
Code: 
You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'WHERE joomla_content.catid = a.category_id ORDER BY text ASC' at line 5 

SQL=SELECT DISTINCT(`joomla_content`.`id`) AS value, `title` AS text 
FROM `joomla_content` AS `joomla_content` 
JOIN WA_CONTENTTYPE_TO_CATEGORY AS A ON A.CONTENTTYPE_ID = 1 
JOIN WHERE joomla_content.catid = a.category_id ORDER BY text ASC

two things to notice:
- a JOIN is prepended to the WHERE clause
- new stuff is all uppercase

Looking to the new code i do not understand why though.
In your new function buildQueryJoin() around line 777+ the Join is detected properly, and i haven't seen anything touched in the prev. where handler.
 
Well i found where the problem coms from. ;-)
the new function buildQueryJoin() fails to dectect the type and condition part.
so what happens is: teh entire expression is treated as the "type" and "condition" remains empty.
So when calling query->join($type, $condition), the joomla query class turns type into uppercase and prepends it to its own JOIN.

Trouble starts around #748, the seperation types/expressions (which are in fact the conditions) do not work.

Around #760 preg_replace() does not detect and remove the join.

I am looking into this now.
 
ok its done and working:

in buildQueryJoin() around #700 add regex for a simple join.
Code: 
			$re =   "(JOIN)" .
					"|" .
					"(LEFT\s+JOIN)" .
					"|" .

around line #762 fix the regex to remove JOIN. Note the change of s+ to s*, because if teh line starts with JOIN, there is no whitspace or so.
Code: 
						$join = preg_replace("#\s*JOIN\s*#i", '', $join);
						$join_types[] = trim($join);


Pls let me know when you have checked these changes and commited to git.

Again thanks for your trouble.
 
Well i just checked the recent github version. Modifications have been done like 3 days ago: "Fixed issue in join's dataConsideredEmpty(), for multi select joins." but the fix i posted 20 days ago to fix the join issue has not made his way into github.

This means that anyone who wants to use that plugin with a simple join will fail, have to find this thread and patch the code himself.
The patch are actually just 2 lines of code as outlined above.

Is there any reason i am not aware of to not fix this great plugin, or is there any special procedure to follow to propose fixes?

Regards Wolfgang
 
You must log in or register to reply here.

Useful links

  • Create a Fabrik Account (Downloads)
  • Create a Forum Account
  • Fabrik Wiki
  • Fabrik 4 Changelog /Install /Upgrade
    • We are in need of some funding.
    More details.

    Thank you.

    Members online

    Total: 145 (members: 2, guests: 143)
    Back
    Top