Re: Reduce palloc's in numeric operations.

Thanks for comments,

> Have to be careful to really not modify the
> operands. numeric_out() and numeric_out_sci() are wrong; they
> call get_str_from_var(), which modifies the argument. Same with
> numeric_expr(): it passes the argument to
> numericvar_to_double_no_overflow(), which passes it to
> get_str_from_var(). numericvar_to_int8() also modifies its
> argument, so all the functions that use that, directly or
> indirectly, must make a copy.

mmm. My carefulness was a bit short to pick up them...

I overlooked that get_str_from_var() and numeric_to_int8() calls
round_var() which rewrites the operand. I reverted numeric_out()
and numeric_int8(), numeric_int2().

Altough, I couldn't find in get_str_from_var_sci() where the
operand would be modified.

The lines where var showing in get_str_from_var_sci() is listed
below.

|  2:get_str_from_var_sci(NumericVar *var, int rscale)
| 21:  if (var->ndigits > 0)
| 23:    exponent = (var->weight + 1) * DEC_DIGITS;
| 29:    exponent -= DEC_DIGITS - (int) log10(var->digits[0]);
| 59:  div_var(var, &denominator, &significand, rscale, true);

The only suspect is div_var at this level, and do the same thing
for var1 in div_var.

|   2:div_var(NumericVar *var1, NumericVar *var2, NumericVar *result,
|  20:  int      var1ndigits = var1->ndigits;
|  35:  if (var1ndigits == 0)
|  47:  if (var1->sign == var2->sign)
|  51:  res_weight = var1->weight - var2->weight;
|  68:  div_ndigits = Max(div_ndigits, var1ndigits);
|  83:  memcpy(dividend + 1, var1->digits, var1ndigits * sizeof(NumericDigit));
| 132:      for (i = var1ndigits; i >= 0; i--)

No line seems to modify var1 as far as I see so I've left
numeric_out_sci() modified in this patch.


Well, I found some other bugs in numeric_stddev_internal.
vN was errorniously freed and vsumX2 in is used as work.
They are fixed in this new patch.

> Perhaps get_str_from_var(), and the other functions that
> currently scribble on the arguments, should be modified to not
> do so. They could easily make a copy of the argument within the
> function. Then the callers could safely use
> set_var_from_num_nocopy(). The performance would be the same,
> you would have the same number of pallocs, but you would get
> rid of the surprising argument-modifying behavior of those
> functions.

I agree with that. const qualifiers on parameters would rule this
mechanically. I try this for the next version of this patch.


> SELECT SUM(col) FROM numtest;
> 
> The execution time of that query fell from about 5300 ms to 4300 ms, ie. about 20%.

Wow, it seems more promising than I expected. Thanks.

regards,

-- 
堀口恭太郎

日本電信電話株式会社 NTTオープンソフトウェアセンタ
Phone: 03-5860-5115 / Fax: 03-5463-5490