Details
-
Type:
Sub-task
-
Status:
Closed
-
Priority:
Major
-
Resolution: Fixed
-
Affects Version/s: 2.0
-
Fix Version/s: 2.0
-
Component/s: Database SQL/XMLDB
-
Labels:None
-
Affected Branches:MOODLE_20_STABLE
-
Fixed Branches:MOODLE_20_STABLE
Description
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 
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