![](https://secure.gravatar.com/avatar/68ff00c1752bf8aede68a05f77ebd28b.jpg?s=120&d=mm&r=g)
Hi Sergei, Thanks for review. Comments follow inline: On 01/28/2018 07:22 PM, Sergei Golubchik wrote:
Hi, Alexander!
On Nov 03, Alexander Barkov wrote:
Hello Sergei,
Please review a patch for MDEV-13790.
It removes the attempt to search the best suitable buffer and just returns the result in "str" passed to val_str().
I'm quite sure this optimization gives nothing. In can be useful only under very rare conditions, but generates so many bugs.
There're bunch of tricks. What do they all do, could you list them?
One handles the case when an argument is a substring of the already concatenated string - this is very weird and rare, I agree.
Other accumulates the result in the first argument's result - this avoids one memcpy per row. Doesn't sound like much, but it's not rare, so should be very easy to benchmark.
In my patch the first argument goes directly to "str" parameter of Item_func_concat::val_str(). There should not be an extra memcpy here.
Yet another doubles the buffer size every time - this might be useful to keep, it easily fits in your new code.
As discussed on IRC, I kept this code. Please find a new version attached.
What are other tricks that concat was using?
Btw, one comment about your patch — please don't call current_thd in a loop.
Fixed.
Regards, Sergei Chief Architect MariaDB and security@mariadb.org