On Wed, Dec 26, 2018 at 03:53:35PM +0900, Amit Langote wrote:
> Note that this check is same as one of the checks performed by
> truncate_check_activity() that's called below, but it errors out on
> this being true, whereas we'd like to ignore such child tables. It
> makes sense to repeat the check here instead of complicating the
> code in truncate_check_activity() to get the behavior we want.
>
> A bit long but clearer I think.
No objections in doing something like that.
> * Maybe, you should rename inh_temp_parent to inh_parent (or
> inh_perm_parent if you want to), because the "temp" in the name makes
> someone looking at this think that the parent not accessible from both
> sessions or that both sessions have their own temporary parent. Of
> course, they can look at setup() and know that that's not the case, but
> it's better to convey that using the naming.
>
> * Related, maybe rename inh_temp_s1/s2 to inh_temp_child_s1/s2.
>
> * Tests s1/s2_update/delete_c should be written such that the query
> appears to try to update/delete other session's data, which doesn't work
> because the other session's child will be skipped. For example:
>
> change: "s1_update_c" { UPDATE inh_temp_parent SET a = 13 WHERE a = 3; }
> to: "s1_update_c" { UPDATE inh_temp_parent SET a = 13 WHERE a IN (3, 5); }
Okay, let's do as you suggest for all these things. Thanks for
looking at the patch set. I will try to finish wrapping that stuff
tomorrow, back-patching the tests on the way.
--
Michael