On Wed, Mar 3, 2021 at 5:50 PM Greg Nancarrow <gregn4...@gmail.com> wrote: > > Asserts are normally only enabled in a debug-build, so for a > release-build that Assert has no effect. > The Assert is being used as a sanity-check that the function is only > currently getting called for INSERT, because that's all it currently > supports.
I agree that assert is only for debug build, but once we add and assert that means we are sure that it should only be called for insert and if it is called for anything else then it is a programming error from the caller's side. So after the assert, adding if check for the same condition doesn't look like a good idea. That means we think that the code can hit assert in the debug mode so we need an extra protection in the release mode. > > > 2. > > In patch 0004, We are still charging the parallel_tuple_cost for each > > tuple, are we planning to do something about this? I mean after this > > patch tuple will not be transferred through the tuple queue, so we > > should not add that cost. > > > > I believe that for Parallel INSERT, cost_modifytable() will set > path->path.rows to 0 (unless there is a RETURNING list), so, for > example, in cost_gather(), it will not add to the run_cost as > "run_cost += parallel_tuple_cost * path->path.rows;" > But the cost_modifytable is setting the number of rows to 0 in ModifyTablePath whereas the cost_gather will multiply the rows from the GatherPath. I can not see the rows from GatherPath is ever set to 0. -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com