Heikki Linnakangas <hlinnakangas@vmware.com> writes:
> * Add init_var_from_num() function. This is the same as
> set_var_from_num_nocopy in the original patch, but it doesn't require
> the caller to have called init_var() first. IMHO this makes the calling
> code slightly more readable. Also, it's now more evident what these vars
> are: the digits array points to original array in the original Datum,
> but 'buf' is NULL. This is the same arrangement that's used in the
> constant NumericVars like const_zero.
init_var_from_num's header comment could use some more work. The
statement that one "must not modify the returned var" is false in some
sense, since for instance numeric_ceil() does that. The key is that you
have to replace the digit buffer not modify it in-place, but most
computational routines do that anyway. Also it might be worth pointing
out explicitly that free_var() is not required unless the var is
modified subsequently. (There are instances of both cases, and it might
not be clear to the reader why.)
> * get_str_from_var() no longer scribbles on its input. I noticed that
> it's always called with a dscale that comes from the input var itself.
> In other words, the rounding was unnecessary to begin with.
Hmm, it may have been necessary once upon a time, but I agree getting
rid of the rounding step is a win now. Suggest adding a comment though
that the var is displayed to the number of digits indicated by its dscale.
Looks good otherwise.
regards, tom lane
