History | Log In     View a printable version of the current page.  
We are currently focused especially on Moodle 2.0, Moodle 1.9.x bugs and Moodle 1.9.x testing.    Confused? Lost? Please read this introduction to the Tracker.
Issue Details (XML | Word | Printable)

Key: MDL-14974
Type: Sub-task Sub-task
Status: Closed Closed
Resolution: Fixed
Priority: Major Major
Assignee: Petr Škoda
Reporter: Eloy Lafuente (stronk7)
Votes: 0
Watchers: 0
Operations

If you were logged in you would be able to see more operations.
Moodle
MDL-14679

reset_columns() implementation...

Created: 25/May/08 10:27 AM   Updated: 26/May/08 12:32 AM
Component/s: Database SQL/XMLDB
Affects Version/s: 2.0
Fix Version/s: 2.0

Participants: Eloy Lafuente (stronk7) and Petr Škoda
Security Level: None
QA Assignee: Eloy Lafuente (stronk7)


 Description  « Hide
Two things about reset_columns()

1) implementation looks buggy. If not table is passed, then the whole $this->columns should be = array() AFAIK. Easy to fix.
2) then, the method itself, I don't know in what situation we are going to use this, apart from all DDL stuff. So, IMO it would be easier to add one param to get_columns($table, $usecache = true). And them with that parameter:

a) if $usecache = true ==> current behaviour (using cache). Normal situation for DML stuff using get_columns()
b) if $usecache = false ==> never use the cache + reset it (for requested table) after execution. And we'd add that second parameter to all the DDL stuff using get_columns().

How does it sound. IMO it's easier that adding reset_columns() here and there. Just one param to control behaviour (cache/no cache).

Ciao :-)

 All   Comments   Change History   Version Control      Sort Order: Ascending order - Click to sort in descending order
Petr Škoda - 25/May/08 05:34 PM
done,
1/ columns are reset automatically when db structure changes in moodle_database->change_database_structure() + in old execute_sql() for now
2/ caching in get_columns() is now optional - new param
3/ table param is not present in reset_columns() anymore - not needed, we reset everything because we would not usually know what changed anyway

please reopen if needed, thanks for the report

Eloy Lafuente (stronk7) - 26/May/08 12:32 AM
Peeeeerfect! Thanks! B-)