Hi, Sergei! 26 Jan 2019 06:27, Sergei Golubchik <[email protected]>:
> > > > > > +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 > Ok, did it. Please see the last update. > > > > > > > + 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. > Well, still quite arguable for me, but ok. I made it in the same manner TABLE::delete_row written -- just moved the method in sql_delete.cc and marked it inline. Is this solution OK for You? With regards, Nikta Malyavin
_______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : [email protected] Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
