Hi, Nikita! On Jan 25, Nikita Malyavin wrote:
+delete from t for portion of othertime from '2000-01-01' to '2018-01-01'; +ERROR HY000: period `othertime` is not found in table +delete from t for portion of system_time from '2000-01-01' to '2018-01-01'; +ERROR HY000: period SYSTEM_TIME is not allowed here
could be just ER_PARSE_ERROR too, I suppose
I think it worth separate error here. ER_PARSE_ERROR will make "right syntax to use near '' at line 1", just like in the above case.
it depends on how you write the grammar :)
What about "for portion of `SYSTEM_TIME` ..."? Is it correct to throw ER_PARSE_ERROR in this case?
Yes, I'd think that You have an error in your SQL syntax; ... use near 'SYSTEM_TIME' at line 1 would be quite reasonable
+ if (likely(!res) && table->triggers) + res= table->triggers->process_triggers(thd, TRG_EVENT_INSERT, TRG_ACTION_BEFORE, true); + + if (likely(!res)) + res = table->file->ha_write_row(table->record[0]); + + if (likely(!res) && table->triggers) + res= table->triggers->process_triggers(thd, TRG_EVENT_INSERT, TRG_ACTION_AFTER, true); + + restore_record(table, record[1]); + return res; +} + +int TABLE::update_portion_of_time(THD *thd, vers_select_conds_t &period_conds, + bool *inside_period)
is it used for UPDATE too? or only for DELETE?
Looks like for DELETE only. Does the naming look confusing?
No, I wanted to ask to move it from a TABLE method to a static function in sql_delete.cc:
static int update_portion_of_time(THD*, TABLE*, vers_select_conds_t&, bool*);
Well, technically moving it to sql_delete will make table_update_generated_fields and period_make_insert extern. I also like current code arrangement by the following reason: these functions make similar operations, so better to store them in one place. I'd actually prefer to have a separate class/module for them, with TABLE::delete_row and maybe some others, but was afraid It'll be not in style the rest code is written -- seems, mysql/mariadb developers prefer not to spawn a lot of files, and to group logical API blocks by comments
Well, yes, and because update_portion_of_time() is a delete-specific functionality, it should be logically with the rest of the delete code and not polluting global namespace and shared objects. And if period_make_insert() is used both for UPDATE and DELETE, then it'd be quite logical to make it extern and use both from UPDATE and from DELETE. Regards, Sergei Chief Architect MariaDB and security@mariadb.org