Development talk:DB layer 2.0 migration docs: Difference between revisions
Line 41: | Line 41: | ||
# I'm not sure I'm clear on what the advantages are in using the "params" array for the "*_sql" functions (e.g. get_records_sql). Also, the text says we '''must''' do it, but I see lots of examples in the already converted code where this hasn't been done. Is it really a '''must do''' or is it more of a '''can do'''? | # I'm not sure I'm clear on what the advantages are in using the "params" array for the "*_sql" functions (e.g. get_records_sql). Also, the text says we '''must''' do it, but I see lots of examples in the already converted code where this hasn't been done. Is it really a '''must do''' or is it more of a '''can do'''? | ||
:You must always use the params array WHEN you have params to pass ;) If the SQL doesn't contain dynamic parameters (subject to SQL injection), we don't need the params array. [[User:Nicolas Connault|Nicolas Connault]] 13:57, 21 July 2008 (CDT) | :You must always use the params array WHEN you have params to pass ;) If the SQL doesn't contain dynamic parameters (subject to SQL injection), we don't need the params array. [[User:Nicolas Connault|Nicolas Connault]] 13:57, 21 July 2008 (CDT) | ||
:Aha! So the purpose is to help make sure that dynamic data used in SQL searches is cleansed? Is that correct?[[User:Mike Churchward|Mike Churchward]] 21:31, 21 July 2008 (CDT) | :Aha! So the purpose is to help make sure that dynamic data used in SQL searches is cleansed? Is that correct? [[User:Mike Churchward|Mike Churchward]] 21:31, 21 July 2008 (CDT) | ||
# The new recordset handling seems odd. We used to use the "rs_fetch_next_record($rs)" function, which as I understood it was optimized so that the entire db records contents weren't stored in the PHP array variable. Now we are replacing it with a "foreach ($rs as $record)". Doesn't that mean that the entire data query results are stored in a PHP variable again? | # The new recordset handling seems odd. We used to use the "rs_fetch_next_record($rs)" function, which as I understood it was optimized so that the entire db records contents weren't stored in the PHP array variable. Now we are replacing it with a "foreach ($rs as $record)". Doesn't that mean that the entire data query results are stored in a PHP variable again? | ||
:The new recordset uses a new feature of PHP5, the Iterator interface. Foreach can then be used on the recordset. (See the bottom of [http://phplens.com/adodb/code.initialization.html the adodb doc] for more info). [[User:Nicolas Connault|Nicolas Connault]] 13:57, 21 July 2008 (CDT) | :The new recordset uses a new feature of PHP5, the Iterator interface. Foreach can then be used on the recordset. (See the bottom of [http://phplens.com/adodb/code.initialization.html the adodb doc] for more info). [[User:Nicolas Connault|Nicolas Connault]] 13:57, 21 July 2008 (CDT) | ||
:Hmmm... Looks like I need to spend some time learning PHP5's features. So, in PHP5, the "foreach" construct unlocks a classes' interator functions if they exist?[[User:Mike Churchward|Mike Churchward]] 21:31, 21 July 2008 (CDT) | :Hmmm... Looks like I need to spend some time learning PHP5's features. So, in PHP5, the "foreach" construct unlocks a classes' interator functions if they exist? [[User:Mike Churchward|Mike Churchward]] 21:31, 21 July 2008 (CDT) |
Revision as of 02:32, 22 July 2008
- The developer reading this article MUST read XMLDB Documentation (at least Introduction + three first Developing section pages) otherwise he's going to ask plenty of questions :) It could be very good that the first line of this article be: "In order to understand this article you need to know how works the database abstraction layer in Moodle (previous to 2.0)".
- API link is broken (=> http://php.moodle.org/ "Server not found")
- We know that some changes need to be done on 2.0. But on what?
All these examples concerns the glossary module. It would be good to indicate it first. When I read it first, I didn't really know what was going to be explained in XMLDB changes. In fact all this section is about "Updating your upgrade.php file for Moodle 2.0". Conclusion: we really need a clear first sentence for the all "XMLDB changes" section. So we know what we are going to read. Once I knew what is about, all was very clear.
- If we don't make any change on DDL code we shouldn't make it as a section/block. The DDL section should be a note. If you remove this section don't forget to update: "Changes below are grouped into 3 main blocks" => "Changes below are grouped into 2 main blocks"
- DML section:
this is about to change all DML code, in all Moodle files? I think it miss an introduction line as well.
- Maybe you should use more the word: YOU should do that, YOU do this, YOU ... The developer will feel more guided.
Note: I wrote this comment on first read.
Note 2: I'm new as dev in Moodle, and I've just discover all the power of Moodle XMLDB reading. All the current XMLDB looks awesome, I don't even talk about the coming 2.0 :)
Jerome mouneyrac 22:22, 22 May 2008 (CDT)
random things
1 I added a marker re add_index or somesuch, is it really renamed add_key or is that just a copy/paste error? If it is correct, needs changing to emphasise that the function has really changed.
2 The section with nothing in it (DDL) should probably be removed :)
3 'Golden' and 'iron' makes no sense at all to me, nor did the explanation. Could we rename these in some way?
4 The example using named params should probably be changed to use better names for the params eg fn, ln - if you're using 'param1' and 'param2' you should use use the ordered one.
5 Some of the examples are unnecessarily wordy - for example, why put the array into a variable then call the function? It only has two elements, you can do it in the same line no problem.
- Wordiness in code samples is perfectly OK, Sam, but you're free to be more terse in your implementations ;) Nicolas Connault 06:59, 3 June 2008 (CDT)
- Yes, but my point is that it makes it look bad for people who are having to change. It's like, OMG! this was one line before and now it's four! wtf I have to define a separate array every time I scratch my backside! why are you making my life a misery, you unfeeling b****rds!!! That kind of thing. :) I don't think it makes it easier to understand to show use of a mostly-unnecessary separate variable. Sam marshall 06:24, 6 June 2008 (CDT)
6 Wow this new system is a fantastic improvement (except for the new IN function which is kind of nasty)
Sam marshall 04:32, 3 June 2008 (CDT)
Why / when do we have to use the params array in the "_sql" functions?
- I'm not sure I'm clear on what the advantages are in using the "params" array for the "*_sql" functions (e.g. get_records_sql). Also, the text says we must do it, but I see lots of examples in the already converted code where this hasn't been done. Is it really a must do or is it more of a can do?
- You must always use the params array WHEN you have params to pass ;) If the SQL doesn't contain dynamic parameters (subject to SQL injection), we don't need the params array. Nicolas Connault 13:57, 21 July 2008 (CDT)
- Aha! So the purpose is to help make sure that dynamic data used in SQL searches is cleansed? Is that correct? Mike Churchward 21:31, 21 July 2008 (CDT)
- The new recordset handling seems odd. We used to use the "rs_fetch_next_record($rs)" function, which as I understood it was optimized so that the entire db records contents weren't stored in the PHP array variable. Now we are replacing it with a "foreach ($rs as $record)". Doesn't that mean that the entire data query results are stored in a PHP variable again?
- The new recordset uses a new feature of PHP5, the Iterator interface. Foreach can then be used on the recordset. (See the bottom of the adodb doc for more info). Nicolas Connault 13:57, 21 July 2008 (CDT)
- Hmmm... Looks like I need to spend some time learning PHP5's features. So, in PHP5, the "foreach" construct unlocks a classes' interator functions if they exist? Mike Churchward 21:31, 21 July 2008 (CDT)