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 secur...@mariadb.org _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp