|
|
|
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 :-)
|
|
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 :-) |
Show » |
|
Sub-task
Closed
Major
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