Hooking up data loading
[Plugin] Awards »

Dragooon

  • I can code! Really!
  • polygon.com has to be one of the best sites I've seen recently.
  • Posts: 1,841
Re: Hooking up data loading
« Reply #45, 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?
The way it's meant to be

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: Hooking up data loading
« Reply #46, 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.
When we unite against a common enemy that attacks our ethos, it nurtures group solidarity. Trolls are sensational, yes, but we keep everyone honest. | Game Memorial

Dragooon

  • I can code! Really!
  • polygon.com has to be one of the best sites I've seen recently.
  • Posts: 1,841
Re: Hooking up data loading
« Reply #47, 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.

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: Hooking up data loading
« Reply #48, 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.

Dragooon

  • I can code! Really!
  • polygon.com has to be one of the best sites I've seen recently.
  • Posts: 1,841
Re: Hooking up data loading
« Reply #49, 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.
Re: Hooking up data loading
« Reply #50, on September 26th, 2011, 12:47 PM »
What do the rest think about it? Especially the query builder example I posted on previous page.

live627

  • Should five per cent appear too small / Be thankful I don't take it all / 'Cause I'm the taxman, yeah I'm the taxman
  • Posts: 1,670
Re: Hooking up data loading
« Reply #51, 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.
A confident man keeps quiet.whereas a frightened man keeps talking, hiding his fear.

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: Hooking up data loading
« Reply #52, 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.

Dragooon

  • I can code! Really!
  • polygon.com has to be one of the best sites I've seen recently.
  • Posts: 1,841
Re: Hooking up data loading
« Reply #53, 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 :).

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: Hooking up data loading
« Reply #54, 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.

Dragooon

  • I can code! Really!
  • polygon.com has to be one of the best sites I've seen recently.
  • Posts: 1,841
Re: Hooking up data loading
« Reply #55, 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.

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: Hooking up data loading
« Reply #56, 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.

Dragooon

  • I can code! Really!
  • polygon.com has to be one of the best sites I've seen recently.
  • Posts: 1,841
Re: Hooking up data loading
« Reply #57, 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.

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: Hooking up data loading
« Reply #58, 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.

Dragooon

  • I can code! Really!
  • polygon.com has to be one of the best sites I've seen recently.
  • Posts: 1,841
Re: Hooking up data loading
« Reply #59, 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.

[Plugin] Awards »