Wedge

Public area => The Pub => Plugins => Topic started by: Dragooon on September 25th, 2011, 10:46 AM

Title: Hooking up data loading
Post by: Dragooon on September 25th, 2011, 10:46 AM
Alright, this has been discussed in private a few months ago, but back then it was less relevant due to the absence of a proper add-on manager. But since Arantor has made a brand new and nothing short of awesome Add-on manager coupled with Nao's enhanced theme skeleton system, this topic is now more relevant than ever.

One of the few remaining major flaw with the current hook system is that queries and data loading from MySQL cannot be hooked in any form unless the result itself is passed as a hook, but that requires creating innumerable amount of hooks. Even then it wouldn't solve the modifying query problem. One easy solution is just to add an ID to every query and pass the string to hooks, so that they can be modified via string manipulation, but that is a very bad idea because it can easily mess up query bad.

Another idea I had is to objectify the query, such that every SQL statement is an individual variable, this way add-ons can just add to those arrays in order to add there own statements.


Update : Upon further discussion I have a more concrete implementation made, here's an update summarizing the current progress. Please read the whole update before providing your opinions, any feedback is appreciated.

The aim of the current solution is to vaguely objectify the queries, by "vaguely" I mean that only the major portion of the query, i.e. "SELCT, WHERE etc" statement's SQL lines are passed. The SQL passed for those statement itself is not objectified. Although possible, doing so would severely limit down the syntax and offer hardly any more advantages.

Basic query functions
wesqlQuery::__construct(string $name, array $sql_params, array $params)
This is the beginning point of any query, an unique name must be passed. "sql_params" is an array containing various SQL structs and their corresponding sql statements. Every individual SQL statement of a SQL struct must be passed as an individual array value. "params" is your traditional query parameter, the parameters are defined in same way as the correct. Example :
new wesqlQuery('example', array(
    
'select' => array('field''other_field'),
    
'from' => array('table'),
    
'where' => array('member = {int:member}'),
), array(
    
'member' => 1,
));
This will produce the fol. SQL query :
Code: [Select]
SELECT field, other_field
FROM table
WHERE member = 1
As you can see, 2 fields are passed as individual array members of the select array value passed to SQL params, same way, multiple where statements can also be done same way
new wesqlQuery('example', array(
    
'select' => array('field''other_field'),
    
'from' => array('table'),
    
'where' => array(
        
'member = {int:member}',
        
'field' = {string:somefieldval}',
    ),
), array(
    '
member' => 1,
    '
somefieldval' => 'test',
));
Will produce this SQL query :
Code: [Select]
SELECT field, other_field
FROM table
WHERE member = 1
    AND field = 'test'

Passing extra options to SQL structs
Some special options such as "SELECT DISTINCT" or "UNION ALL" can also be used, detailed documentation for every struct and their extra options can be found later. In order to pass extra options to structs, the struct array should contain 2 child arrays, first one should contain the SQL statements, second one should contain the parameters passed.
Example(Only shows the select part, rest is assumed) :
new wesqlQuery('example', array(
    
'select' => array(
        array(
'field''other_field'),
        array(
'distinct' => true),
    ),
));

wesqlQuery::bindSQLParam(string $name, array $value, array $extraOptions)
Instead of passing all the SQL params through __construct, one can bind individual SQL structs using this method. Please remember that values are not replaced by this method but are appended to the existing methods.

Example :
$query->bindSQLParam('where''member = 1');
extraOptions behave similarly to above
$query->bindSQLParam('select', array('test''field'), array('distinct' => true));
wesqlQuery::bindParam(string $name, mixed $value)
Like above, query parameters can also be added through this method.

wesqlQuery::execute()
Executes a query, throws an error if any.

wesqlQuery::fetch(bool $run_callbacks = true, array $hook_parameters)
Fetches the next row, if run_callbacks is set to true it'll also run the row through all the callbacks and then return the resulting row. More on callbacks and hooks later.

Hooking and modifying queries
The major point of objectifying queries is to be able to modify them through hooks without running string functions and/or conflicting with others. For every query made, a new hook called "query_<name>" is called as soon as a query is executed. Only one parameter is passed and that is the instance of the query itself, from there you cannot execute the query again as that will throw an error. You have access to all the bind functions as well as other helper functions, but fetch or other functions which rely on result would not be available.

Here's an example of a query, to which an add-on adds a new SELECT field as well as a WHERE clause
new wesqlQuery('sample', array(
    
'select' => array('field'),
    
'from' => array('table'),
));

add_hook('query_sample''query_sample_hook'false);

function 
query_sample_hook(wesqlQuery $query)
{
    
$query->bindSQLParam('select''another_field');
    
$query->bindSQLParam('where''field = {string:var}');
    
$query->bindParam('var''test');
}

What about results?
Modifying query is useless until it can actually be accessed by the template via the result that is stored. Worry not, we have result callbacks. Each result callback is passed every row fetched individually, and it's supposed to modify the row in a nice and suitable manner.

Parameters passed : array &$row, array $original_row + Any other parameter passed by the query through fetch's second argument

Result callback must use the second argument for reference and must modify the first argument, which is referenced to the original variable and is automatically picked up. The callback is not required to return anything.

Example (Continuing from the previous one) :
function query_sample_hook(wesqlQuery $query)
{
    
$query->bindSQLParam('select''another_field');
    
$query->bindSQLParam('where''field = {string:var}');
    
$query->bindParam('var''test');
    
$query->addResultCallback('query_sample_result_callback');
}
function 
query_sample_result_callback(&$row$original)
{
    
$row['var'] = $original['another_field'];
}

Guidelines for general query fetching
The general flow of query fetching is that a query is run, a while loop is executed fetching the rows and the information from rows are added to the main array in a differently formatted way than originally fetched. The main array is then passed on to templates.

Now this has one fundamental flaw when implementing with this query model, which is if a new column is fetched, it will not be added since a new array is formed and just be there hanging. The simple solution is that the initial assignation should be handled by a result callback itself, and then the row would be carried over and modified by later addons. Example :
$query = new wesqlQuery('sample', array(
    
'select' => array('field''some_field'),
    
'from' => 'table',
));
$query->addResultCallback('query_sample_initial_result');
$values = array();
while (
$row $query->fetch())
    
$values[] = $row;

function 
query_sample_initial_result(&$row$original)
{
    
$row = array(
        
'id' => $row['field'],
        
'field' => $row['some_field'],
    );
}
As you can see, the row is modified by a callback and then appended to the main array($values).

An addon can extend this as :
add_hook('query_sample''query_sample_hook'false);
function 
query_sample_hook(wesqlQuery $query)
{
    
$query->bindSQLParam('select''another_field');
    
$query->bindSQLParam('where''field = {string:var}');
    
$query->bindParam('var''test');
    
$query->addResultCallback('query_sample_result_callback');
}
function 
query_sample_result_callback(&$row$original)
{
    
$row['var'] = $original['another_field'];
}

Now the values in $values array would have another field called "var" having values from "another_field" in the MySQL table.

Miscellaneous
Sub-queries
Sub queries are also important to handle properly, instead of being passed as a string, they are now treated as an individual parameter with type called "query". Another wesqlQuery is passed for sub-queries. Add-ons can also hook into this query same way as above, but result callbacks will not be called(Instead they'll be called for the main query).

Example :
new wesqlQuery('example', array(
    
'select' => array('field''other_field'),
    
'from' => array('table'),
    
'where' => array(
        
'member = {query:member_query}',
    ),
), array(
    
'member_query' => new wesqlQuery('example_member', array(
        
'select' => array('id_member'),
        
'from' => '{db_prefix}members',
    )),
));

Unions
Unions are treated similarly to sub-queries, 2 wesqlQuery objects must be passed to a main query under union SQL struct
Example :
new wesqlQuery('example', array(
    
'union' => array(new wesqlQuery(....), new wesqlQuery(.....)),
));

A fairly complex builder example
Why not?

For the fol. SQL query :
Code: [Select]
(
SELECT DISTINCT t1.field1, t2.field2
FROM wedge_table1 AS t1
INNER JOIN wedge_table2 AS t2 ON (t1.field3 = t2.field4)
INNER JOIN wedge_table4 AS t4 ON (t4.field7 = t2.field7)
LEFT JOIN wedge_table3 AS t3 ON (t3.mem = t2.mem)
WHERE t1.member = 5 AND (t2.field6 = 0 OR t3.field6 = 0)
GROUP BY t1.field1
HAVING t1.field1 = 'test'
ORDER BY t1.member ASC
LIMIT 1, 50
) UNION ALL (
SELECT IFNULL(m.name, mem.name) AS name, mem.id_member
FROM wedge_mems AS m
INNER JOIN wedge_members AS mem ON (m.id_member = mem.id_member)
WHERE m.id_member IN (
SELECT p.id_member
FROM wedge_players AS p
WHERE p.field IN ('hockey', 'cricket')
)
ORDER BY mem.id_member DESC
)
The fol. code can be used
new wesqlQuery('example', array(
'union' => array(
new wesqlQuery('example_sub1', array(
'select' => array(
array('t1.field1''t2.field2'),
array('distinct' => true),
),
'from' => '{db_prefix}table1 AS t1',
'inner_join' => array(
'{db_prefix}table2 AS t2 ON (t1.field3 = t2.field4)',
'{db_prefix}table4 AS t4 ON (t4.field7 = t2.field7)',
),
'left_join' => '{db_prefix}table3 AS t3 ON (t3.mem = t2.mem)',
'where' => array(
't1.member = {member}',
'(t2.field6 = 0 OR t3.field6 = 0)',
),
'group_by' => 't1.field1',
'having' => 't1.field1 = {field1}',
'order_by' => 't1.member ASC',
'limit' => '1, 50',
), array(
'member' => array('int'5),
'field1' => array('string''test'),
)
),
new wesqlQuery('example_sub2', array(
'select' => array('IFNULL(m.name, mem.name) AS name',  'mem.id_member'),
'from' => '{db_prefix}mems AS m',
'inner_join' => '{db_prefix}members AS mem ON (m.id_member = mem.id_member)',
'where' => 'mem.id_member IN ({mem_player_query})',
'order_by' => 'mem.id_member DESC',
), array(
'mem_player_query' => array('query', new wesqlQuery('example_sub2_sub', array(
'select' => 'p.id_member',
'from' => '{db_prefix}players AS p',
'where' => 'p.pfield IN ({fields})',
), array(
'fields' => array('array_string', array('hockey''cricket')),
)
)),
)
)
),
));

I've attached the current Class-DBQuery.php containing wesqlQuery and other required classes.
Title: Re: Hooking up data loading
Post by: Arantor on September 25th, 2011, 11:19 AM
Well, I like the idea and general execution. I don't think I'd attach it to every query, I'd leave a number of queries alone with the existing infrastructure, without any need to expand *every* query.

Then it's just a case of extending the add-on manager to handle query hooks but that's more dependent on the query architecture to be honest.
Title: Re: Hooking up data loading
Post by: Dragooon on September 25th, 2011, 11:30 AM
What kind of query would you attach it to?
Title: Re: Hooking up data loading
Post by: Arantor on September 25th, 2011, 11:49 AM
The ones people actually want to modify, like the monster in loadMemberData, or the one for getting topics, or even the messages request query.

Stuff like the permissions loading queries wouldn't need to be hook able and would in fact be counterproductive to do so because the revised design I have in mind would do away with the need to actually hook onto it, and just use the data itself (like using the current hook in SMF, except with the facility to superset it)
Title: Re: Hooking up data loading
Post by: Dragooon on September 25th, 2011, 11:57 AM
Okay, that makes sense. What do you think about the hooking? I still have to think how to handle the "different" style of loading that we see in loadMemberContext, prepareDisplayContext etc, but I don't see it being much more difficult than it already is.
Title: Re: Hooking up data loading
Post by: Arantor on September 25th, 2011, 12:12 PM
Well, either you do it that way, or expressly call a hook for each query and each result (as in by hand)

What occurs to me is that even if you extend a query, you will probably have to add a hook somewhere to make use of the result, which almost has to be case dependent.

As an example, the subs-board index queries to get boards. While you can extend the query and do something with the result directly, you still have to put the result into the relevant array, and maybe even something in the template to be able to make use of that array value.

On the other hand, if you extend the query in loadMemberData, the base information is still available in $user_profile even if you don't hook it in loadMemberContext.

Trouble is, I don't think there's a nice way to deal with it, except on a case by case basis.
Title: Re: Hooking up data loading
Post by: Dragooon on September 25th, 2011, 12:22 PM
Quote
Trouble is, I don't think there's a nice way to deal with it, except on a case by case basis.
I'm sure it can be dealt with in a nice way, so that case by case basis is not required. Although it'll require some tricky handling, it can still be made part of the whole system.
Title: Re: Hooking up data loading
Post by: Arantor on September 25th, 2011, 12:37 PM
Making it not case by case requires not only standardising the queries but how the query data is used, cf. Display vs Subs-BoardIndex vs loadMemberData.
Title: Re: Hooking up data loading
Post by: Dragooon on September 25th, 2011, 12:38 PM
I just went over loadMemberData and loadMemberContext as well as getBoardIndex, they fit perfectly with my query model. So does prepareDisplayContext. Would anyone want me to actually code the code?
Title: Re: Hooking up data loading
Post by: Arantor on September 25th, 2011, 12:42 PM
I'd love to see it :)
Title: Re: Hooking up data loading
Post by: Dragooon on September 25th, 2011, 12:44 PM
The query class, implementation or both? I'd be happy to do both but in case you want to see something else earlier. I'll start in 15 minutes or so.
Title: Re: Hooking up data loading
Post by: Arantor on September 25th, 2011, 03:09 PM
Well, it's all interrelated, so I'd love to see both, though I'm conscious that I won't be able to really try it out yet.
Title: Re: Hooking up data loading
Post by: Dragooon on September 25th, 2011, 03:17 PM
I'll attach the files, you can see how easy or difficult it is to code them. Give me a few hours, quite busy with other stuff. Apparently they preponed my IT exam to be held tomorrow, ah well, wasn't gonna study anyway.
Title: Re: Hooking up data loading
Post by: Arantor on September 25th, 2011, 03:19 PM
There's really no rush, I'm on my iPad now until Sunday due to family stuff, so unlikely to sit down in front of a desktop before then.
Title: Re: Hooking up data loading
Post by: Dragooon on September 25th, 2011, 10:47 PM
I haven't had this kind of rush since quite a while, feels good to be back. Attached are the 2 files, Class-DBQuery.php and Subs-BoardIndex containing the roughest, most brutal test this class can face.

Now I haven't handle getBoardIndex the way it's meant to be handled, simple reason being that I couldn't figure out a way to. But that doesn't make it less extensible, all the parameters are still tossed around and it is still extensible, but it is sort of a "special case" in terms of implementation, but I didn't need to make any sort of adjustment in the query class itself. This is because of the way things are set, the categories and this_category array are tossed like burger on a pan, maybe I missed it, but it works fairly well.

The definitions for SQL Params are probably incomplete in the Class-DBQuery file, but contains the most common ones used : INSERT INTO, VALUES, SELECT, FROM, LEFT JOIN, INNER JOIN, WHERE, GROUP BY, HAVING, ORDER BY, LIMIT, UNION(I'm not too sure about this)
One's that still need to be added from the top of my head : REPLACE INTO, INSERT IGNORE INTO, RIGHT JOIN, and quite a few more.

I'll leave it at this, been coding for over 8 hours and it's 2 AM ATM.
Posted: September 25th, 2011, 10:40 PM

Okay, some technicalities, I've made params to be {param} now instead of {type:param}, I've moved type out of param to PHP declaration. Makes more sense and is probably more extensible. I've no idea.

I have also removed query_<name>_result hook in favour of in-line callback handling, this allows control over the order of flow as some functions need to be executed first.
Posted: September 25th, 2011, 10:44 PM

One more thing, I believe getBoardIndex is only one of it's kind, prepareDisplayContext and the likes are far more traditional. Maybe loadMemberContext is similar, I haven't dug into it.
Title: Re: Hooking up data loading
Post by: Arantor on September 25th, 2011, 10:49 PM
I'm tempted to say that INSERT/REPLACE should not be extended in this way but have the parameter list and rows passed to a hook instead to be manipulated (there are no raw INSERTs carried out in the source, they are all done by wesql::insert, and I'd suggest we leave it that way - it is cleaner to do that by not having add-on authors having to deal with the queries' parameterisation)


Also, prepareDisplayContext is pretty unique, everywhere else just processes rows into an array of some form. getBoardIndex is atypical, though, but it's still ultimately running a query and pulling rows into an array.
Title: Re: Hooking up data loading
Post by: Dragooon on September 25th, 2011, 10:52 PM
Quote from Arantor on September 25th, 2011, 10:49 PM
I'm tempted to say that INSERT/REPLACE should not be extended in this way but have the parameter list and rows passed to a hook instead to be manipulated (there are no raw INSERTs carried out in the source, they are all done by wesql::insert, and I'd suggest we leave it that way - it is cleaner to do that by not having add-on authors having to deal with the queries' parameterisation)
Fair enough.
Quote from Arantor on September 25th, 2011, 10:49 PM
Also, prepareDisplayContext is pretty unique, everywhere else just processes rows into an array of some form. getBoardIndex is atypical, though, but it's still ultimately running a query and pulling rows into an array.
But it fits into the norm far better than getBoardIndex, It pretty much acts like a result callback and hence is much easier to implement in the flow.
Title: Re: Hooking up data loading
Post by: live627 on September 25th, 2011, 11:00 PM
Quote from Arantor on September 25th, 2011, 10:49 PM
I'm tempted to say that INSERT/REPLACE should not be extended in this way but have the parameter list and rows passed to a hook instead to be manipulated (there are no raw INSERTs carried out in the source, they are all done by wesql::insert, and I'd suggest we leave it that way - it is cleaner to do that by not having add-on authors having to deal with the queries' parameterisation)
Could the fifth (IIRC) parameter (for index) be used for the hook name?
Quote
Also, prepareDisplayContext is pretty unique
As I recall, it has one sister, the search. It works the same way. Do they do that for performance?
Title: Re: Hooking up data loading
Post by: Arantor on September 25th, 2011, 11:07 PM
Well, the old last parameter was for stating keys; that's not needed in MySQL as far as I know if the table is set up properly, so I don't see why not off-hand.

The reason is less for pure performance and more memory: displaying 20 posts at a time is potentially heavyweight if you have to hold them all in memory at once, so they iterate that way to avoid shunting that data about in memory, and on the side, less processing effort.

What I find curious is that print-page does not do that.
Title: Re: Hooking up data loading
Post by: Nao on September 25th, 2011, 11:30 PM
An oversight.
Just like Recent Posts... No?
Title: Re: Hooking up data loading
Post by: Arantor on September 25th, 2011, 11:31 PM
Not sure, to be honest.
Title: Re: Hooking up data loading
Post by: Nao on September 25th, 2011, 11:36 PM
Google likes topic pages more, so they optimized that. It's for the long run. Rarely accessed pages won't bring your server down. Makes sense to me...
Title: Re: Hooking up data loading
Post by: Arantor on September 25th, 2011, 11:39 PM
I still don't think it's about speed, it's more about not running out of memory.
Title: Re: Hooking up data loading
Post by: Dragooon on September 26th, 2011, 12:34 AM
It is about memory, it can't be faster than simple fetching. Any comments about my code?
Title: Re: Hooking up data loading
Post by: live627 on September 26th, 2011, 01:00 AM
I thought I did...
Title: Re: Hooking up data loading
Post by: Dragooon on September 26th, 2011, 07:22 AM
Quote from live627 on September 26th, 2011, 01:00 AM
I thought I did...
What? I am not able to place this in context.
Title: Re: Hooking up data loading
Post by: live627 on September 26th, 2011, 07:45 AM
What I tried to say is I thought I already commented on your code posted in this thread.
Title: Re: Hooking up data loading
Post by: Dragooon on September 26th, 2011, 08:32 AM
Where?
Title: Re: Hooking up data loading
Post by: Dragooon on September 26th, 2011, 09:02 AM
I've been thinking about the whole sub-queries and I've come up with a fairly good solution, I'll treat each of them as separate query and then merge them in the main instance. This'll also properly fix UNION problems, as it is being used in Display.
Title: Re: Hooking up data loading
Post by: Arantor on September 26th, 2011, 09:06 AM
Since when was UNION used in display?
Title: Re: Hooking up data loading
Post by: Dragooon on September 26th, 2011, 09:09 AM
Line 324. Want me to post code?
Title: Re: Hooking up data loading
Post by: Arantor on September 26th, 2011, 09:15 AM
Oh hahaha, yes, now I remember. I think I wrote that! :lol:
Title: Re: Hooking up data loading
Post by: Dragooon on September 26th, 2011, 09:28 AM
Here is how UNION syntax looks, what do you think?

http://pastebin.com/s1vkJkMW
Title: Re: Hooking up data loading
Post by: Arantor on September 26th, 2011, 09:36 AM
I like it provided that it only executes a single query otherwise there's no benefit to using a UNION.
Title: Re: Hooking up data loading
Post by: Dragooon on September 26th, 2011, 09:37 AM
Quote from Arantor on September 26th, 2011, 09:36 AM
I like it provided that it only executes a single query otherwise there's no benefit to using a UNION.
Yes ofcourse, it just combines those 2 queries into one and runs it.
Title: Re: Hooking up data loading
Post by: Dragooon on September 26th, 2011, 10:14 AM
So I've been focusing on the query builder to accommodate even the most complex queries, and here's what I've had so far

For a query like this :
http://pastebin.com/LXudtCx2 (Completely made up, just to show all the SQL possibilities, not meant to make actual sense)
It can be done like this :
http://pastebin.com/dVvN11JQ
Title: Re: Hooking up data loading
Post by: Arantor on September 26th, 2011, 10:21 AM
As live asked, how would someone add multiple inner joins, or left joins or complex wheres with and/or combinations? (e.g where a=1 and (b=2 or c=3))
Title: Re: Hooking up data loading
Post by: Dragooon on September 26th, 2011, 10:31 AM
Quite simple,

Query : http://pastebin.com/X2YqJ76L
PHP : http://pastebin.com/b73Enc5b
Title: Re: Hooking up data loading
Post by: Arantor on September 26th, 2011, 10:42 AM
That's what I suspected. Can I suggest, then, that (at the very least), WHERE, and the JOINS should always be supplied as arrays to avoid add-ons having to test and coerce values to array just so they can add them in?
Title: Re: Hooking up data loading
Post by: Dragooon on September 26th, 2011, 10:44 AM
Even if a string is supplied, internally it's converted into an array. An Add-on wouldn't need to test anything in order to add into the existing values.
Title: Re: Hooking up data loading
Post by: Arantor on September 26th, 2011, 10:45 AM
Ah, OK, then it's all good :)
Title: Re: Hooking up data loading
Post by: Dragooon on September 26th, 2011, 11:11 AM
What do you think of the syntax, ease of use etc?
Title: Re: Hooking up data loading
Post by: Arantor on September 26th, 2011, 11:18 AM
I don't think the syntax needs to change at all (to be honest I'm not sure what else you could do with it anyway) and as for ease of use, I can't see any comparable way of making big queries extensible - it's harder to use than a bare query, until you're used to it, but it's extensible in a way that bare queries aren't.

And since I don't envisage most add-ons having to write them I doubt it'll be a problem, provided there is sufficient documentation and examples of using said setup to extend queries.
Title: Re: Hooking up data loading
Post by: Dragooon on September 26th, 2011, 11:28 AM
Quote from Arantor on September 26th, 2011, 11:18 AM
I don't think the syntax needs to change at all (to be honest I'm not sure what else you could do with it anyway) and as for ease of use, I can't see any comparable way of making big queries extensible - it's harder to use than a bare query, until you're used to it, but it's extensible in a way that bare queries aren't.
Thanks, I've been trying to keep the syntax as simple as possible.
Quote from Arantor on September 26th, 2011, 11:18 AM
And since I don't envisage most add-ons having to write them I doubt it'll be a problem, provided there is sufficient documentation and examples of using said setup to extend queries.
I believe only some major add-ons would like to use those queries in some major part, but even then that'd be rare. Using the hooks to extend existing queries would be a lot more common, especially around loadMemberData, Display and a few other areas. Still, providing documentation won't be hard for this. I've tried to keep the structure logical and easy to understand. Plus it offers the ability to declare custom data types(Something which should be backported to traditional queries honestly).
Title: Re: Hooking up data loading
Post by: Arantor on September 26th, 2011, 11:46 AM
I can, for example, see my adding a couple of hook able queries into WedgeDesk for my own use later.

But as far as 'custom datatypes', what do you have in mind? Data types that have their own rules and/or validation but that ultimately fit into standard MySQL typing? (e.g. WHERE column = {email:address} and inject address in?)
Title: Re: Hooking up data loading
Post by: Dragooon on September 26th, 2011, 11:50 AM
Yeah, as that can help validation. Currently it'll require modification to source and adding a new case under the callback function in wesql class, whereas wesqlQuery would just require a declaration of a new class called wesqlParam_email which can contain the validation and formatting functions required.

Something I'm still thinking about is the parameter syntax, I changed it so that the data type is declared at the PHP code instead of SQL(Query builder in this case), you think I should keep it the old way?
Title: Re: Hooking up data loading
Post by: Arantor on September 26th, 2011, 12:10 PM
I'm pretty enthusiastic about validation at the best of times, but it's not often that we do selects or similar that need anything other than int, string, or an array of ints or strings. Might I suggest, then, that we do that validation in the insertion routine instead?

It's also a performance thing: I'm not enthusiastic about making the process of physically querying any slower than it needs to be, and already there's a significant overhead vs a bare mysql_query call. It isn't a killer because we rarely make beyond 15 queries per page but it is something to bear in mind.

That said, insertion generally is something that happens slightly less often, so I'm not bothered by the notion of expanding the type validation there.[1] And yes, there's no reason we can't use classes there for that.

As for the parameterising of data, I can see both sides of it. On the one hand, your method is much closer to classic query parameters, it also limits the clutter in the query itself, while putting the validation in the SQL does make it easier for me personally write the queries, because when I'm phrasing the SQL, I am conscious of getting the type of variable right.

I'm not fussed either way as a result but whatever is used, it should be consistent.
 1. Just to really fuck with your mind, though, consider how bigint would be dealt with: true bigint cannot be dealt with actually as an int, which means the times I have done it, I've done it in a string.
Title: Re: Hooking up data loading
Post by: Dragooon on September 26th, 2011, 12:19 PM
Quote from Arantor on September 26th, 2011, 12:10 PM
I'm pretty enthusiastic about validation at the best of times, but it's not often that we do selects or similar that need anything other than int, string, or an array of ints or strings. Might I suggest, then, that we do that validation in the insertion routine instead?

It's also a performance thing: I'm not enthusiastic about making the process of physically querying any slower than it needs to be, and already there's a significant overhead vs a bare mysql_query call. It isn't a killer because we rarely make beyond 15 queries per page but it is something to bear in mind.
In the end insertion is also validated at the same place any other query is, and they follow the same int, string, array_int etc ruleset. What I originally said was that expanding these rulesets is much easier than before as now new data types can be declared outside of source.
Title: Re: Hooking up data loading
Post by: Arantor on September 26th, 2011, 12:25 PM
That's the thing; I don't especially want to expand the validation at point of general query, but I would do so happily at the point of data insertion.
Title: Re: Hooking up data loading
Post by: Dragooon on September 26th, 2011, 12:29 PM
Quote from Arantor on September 26th, 2011, 12:25 PM
That's the thing; I don't especially want to expand the validation at point of general query, but I would do so happily at the point of data insertion.
Fair enough, no one needs to.
Title: Re: Hooking up data loading
Post by: Dragooon on September 26th, 2011, 12:47 PM
What do the rest think about it? Especially the query builder example I posted on previous page.
Title: Re: Hooking up data loading
Post by: live627 on September 27th, 2011, 12:14 AM
Quote from Dragooon on September 26th, 2011, 12:47 PM
Especially the query builder example I posted on previous page.
Ah, that was what you wanted before. I thought you wanted comment on the code in the OP... eheh.

I like it, I really do.. :) And to comment on the usability: it's no less something for SMF coders to get used to than the new db structure when they  upgraded their mods from 1.1 to 2.0.
Title: Re: Hooking up data loading
Post by: Arantor on September 27th, 2011, 12:37 AM
You did get some coders just porting their mods by just converting db_query calls to $smcFunc calls, which is lazy and avoids making use of the facilities and the purpose for which they were designed.

Still, I'm comfortable with having differences, hopefully it means we can encourage good coding.
Title: Re: Hooking up data loading
Post by: Dragooon on September 27th, 2011, 11:50 AM
I've updated the first post with latest Class-DBQuery.php along with some documentations. Feedback is appreciated :).
Title: Re: Hooking up data loading
Post by: Arantor on September 27th, 2011, 12:55 PM
Looks good to me :) I can conceive of some nasty situations with this, though, most notably about multi binding the same table unnecessarily.

For any given query, if a table is joined, two things could occur. Firstly, it's possible that users join the same table (with the same criteria) multiple times, especially if the list of tables isn't available to the binding method (can't see right now), since I'm not 100% clear how MySQL will optimise that case.

Worse, someone rebinds a table with an existing alias but with different criteria, e.g. INNER JOIN table2 AS t2 ON (t1.id = t2.id) INNER JOIN table2 AS t2 ON (t2.id = t3.id)


Just as a note to readers: this process will not be necessary on every query and unless you want to make queries hook able (or are using hook able queries), you won't need to worry about this and can just use the standard method.
Title: Re: Hooking up data loading
Post by: Dragooon on September 27th, 2011, 01:29 PM
I don't think there's a good way to avoid multi binding, and upon testing the developer would probably get the error and fix it so I don't see much of a problem anyway.
Title: Re: Hooking up data loading
Post by: Arantor on September 27th, 2011, 01:33 PM
Multi binding is likely to occur betweeen multiple add-ons that use the same hook - not one on its own vs Wedge, so I doubt the dev would see it until real users had issues.

Is there a way in the hook to examine the things already attached to the query? If so, an add-on dev can examine what's there and add it if not already present, or reuse what is there if they so choose.
Title: Re: Hooking up data loading
Post by: Dragooon on September 27th, 2011, 01:41 PM
Yeah I realised that a few minutes after I posted my reply. There are, potentially, 2 ways to resolve this that I can think of

1) Parametrize all the constructs instead of just SQL statements, that way we can match or equalities
2) Do some string operations/regex operations. Extract table and alias and match them against any other one.

We can always pass this information to add-ons if we can get that out, although I doubt add-ons will really implement a set of if/elses.
First option restricts down SQL quite a bit. Second one is unreliable.
Title: Re: Hooking up data loading
Post by: Arantor on September 27th, 2011, 01:54 PM
I honestly wouldn't sweat too much. Make the information available to add-on devs, from the query object instance, in some fashion. If they want to check, that's up to them, if they don't, again that's up to them.

What I'd imagine is that very often the joins that are already in place will suffice, and that add-on authors will avoid extra joins if necessary. I'm not bothered with trying to hold the authors' hands too much, but provide them the tools to avoid problems, I can't make them use those tools.
Title: Re: Hooking up data loading
Post by: Dragooon on September 27th, 2011, 06:49 PM
I'll add some traditional getters and setters for emergency reasons only, if required an add-on author can do some checks of his own. Most of the time it's generally a new field in SELECT anyway.
Title: Re: Hooking up data loading
Post by: Arantor on September 27th, 2011, 06:56 PM
Exactly, I'm expecting that to be the case most of the time, but for the occasions it isn't, having a getter to see what's been added would be useful. Not sure a setter is needed though.
Title: Re: Hooking up data loading
Post by: Dragooon on October 2nd, 2011, 03:02 PM
A setter is just in case scenarios. So, any further thoughts about this? Any objections by anyone(Especially Nao since he hasn't said much about this)?
Title: Re: Hooking up data loading
Post by: Nao on October 2nd, 2011, 03:24 PM
I am 1500 lines late on my plugin code review. I don't even know how it works...
Title: Re: Hooking up data loading
Post by: Arantor on October 2nd, 2011, 04:11 PM
Quote from Nao on October 2nd, 2011, 03:24 PM
I am 1500 lines late on my plugin code review. I don't even know how it works...
What, the plugin code or this?
Title: Re: Hooking up data loading
Post by: Nao on October 2nd, 2011, 06:16 PM
ManagePlugins and other things... Your commits.
Title: Re: Hooking up data loading
Post by: Arantor on October 2nd, 2011, 06:25 PM
That was really why I documented the Mechanics thread, to explain how it worked, both at the author end and the technical end... :/

What about it isn't so clear?
Title: Re: Hooking up data loading
Post by: Nao on October 2nd, 2011, 06:42 PM
I haven't read anything yet.
Title: Re: Hooking up data loading
Post by: Arantor on October 3rd, 2011, 05:51 PM
Well, I don't think I can do too much more work on the plugin manager (other than minor stuff) without adding this before long, simply because most of the hooks I'd really want to add, well, depend on this.

I'm not imagining any problems off it right now, I'll have another read of the code later on tonight, then look at playing with it in a real environment unless anyone has anything else to add?
Title: Re: Hooking up data loading
Post by: Dragooon on October 18th, 2011, 07:56 AM
Updated the first post with a few bug fixes.
Title: Re: Hooking up data loading
Post by: Dragooon on May 30th, 2012, 07:09 PM
Bomp since Arantor brought it up.

PS : High court banned pastebin in my country...WTF?
Title: Re: Hooking up data loading
Post by: Nao on May 30th, 2012, 07:47 PM
Apparently pastebin is also used for communication between software pirates... Well, that's what Google says?!
Title: Re: Hooking up data loading
Post by: Arantor on May 30th, 2012, 07:51 PM
Well, more for boasting about when places get hacked into and sharing all the details of who did so :/
Title: Re: Hooking up data loading
Post by: Dragooon on May 31st, 2012, 12:03 PM
Quote from Arantor on May 30th, 2012, 07:51 PM
Well, more for boasting about when places get hacked into and sharing all the details of who did so :/
Apparently it's to do with preventing piracy. A company convinced the court that blocking these sites (pastebin, vimeo, dailymotion and a plethora of torrent sites) can reduce piracy. Fine, I see some logic in blocking torrent sites, but pastebin, Vimeo and DM are fairly legit video sharing sites...
Title: Re: Hooking up data loading
Post by: Nao on May 31st, 2012, 03:16 PM
Yeah, they are...
Title: Re: Hooking up data loading
Post by: live627 on June 15th, 2013, 12:47 AM
Bumping this to the top. What needs done to wesqlQuery other than fixing it up to use mysqli before it can be included? (Shouldn't this be in the team board?)
Title: Re: Hooking up data loading
Post by: Arantor on June 15th, 2013, 12:51 AM
Last point first, isn't it just a wrapper around wesql::query anyway? If so, it already uses MySQLi by nature. ;)

I don't remember, to be honest. I still like the idea and I can't remember any reason why I didn't get it incorporated before.
Title: Re: Hooking up data loading
Post by: live627 on June 15th, 2013, 01:03 AM
Even though it extends wesql, it it does its own queries instead of calling wesql for its needs. Sounds silly to me and should be changed.
Title: Re: Hooking up data loading
Post by: Arantor on June 15th, 2013, 01:04 AM
I really don't remember enough about it :( Need to find some time to re-read and re-digest everything, but I'm only too aware I'm a deadline now for what I can get done for Wedge.
Title: Re: Hooking up data loading
Post by: Dragooon on June 15th, 2013, 06:54 AM
Quote from live627 on June 15th, 2013, 01:03 AM
Even though it extends wesql, it it does its own queries instead of calling wesql for its needs. Sounds silly to me and should be changed.
I had a good reason for it. I don't know what it was.
Title: Re: Hooking up data loading
Post by: live627 on June 15th, 2013, 08:17 AM
Then why extend wesqll anyway?
Title: Re: Hooking up data loading
Post by: Dragooon on June 15th, 2013, 10:22 AM
Now I remember, it was for $_db_con, it's a protected property of wesql and some error handling (although that can easily be replaced). Otherwise the class was meant to be independent (I didn't want to rely on existing functions, even though it represents them in a lot of ways).