Closed cstiborg closed 11 months ago
Thanks for the PR, I agree that it looks like the right thing to do, i.e. I don't see why should we have an ever-growing indicators_
vector, but I don't understand how can just clearing it, without adjusting any indices for accessing it anywhere, not break something: it looks like either the indices into it were wrong before (always a possibility, of course) or they must be wrong now.
But OTOH I don't actually understand where is this indicators_
actually used at all. It is filled in statement_impl::bind(values & values)
, but where is it used then? I wonder if we could just remove this completely, i.e. could it be a leftover from some earlier version? Or am I missing something obvious here?
Just my two cents:
But OTOH I don't actually understand where is this indicators_ actually used at all. It is filled in statement_impl::bind(values & values), but where is it used then? I wonder if we could just remove this completely, i.e. could it be a leftover from some earlier version? Or am I missing something obvious here?
That's what I wondered about as well. But I think that statement_impl::indicators_
serves as kind of cleanup bucket for values::indicators_
because the entries don't seem to be cleared anywhere else. But the reason for this indirection is unclear to me as well.
That's what I wondered about as well. But I think that
statement_impl::indicators_
serves as kind of cleanup bucket forvalues::indicators_
because the entries don't seem to be cleared anywhere else.
Ah, indeed, thanks, you're absolutely right.
But the reason for this indirection is unclear to me as well.
I have no idea what is the life cycle of values
supposed to be, so I guess it might be too short?
Anyhow, I'll merge this PR for now, thanks again to both of you!
Merged in the commit above.
This small patch assures that the indicators_ vector is cleared after its content is deleted in bind_cleanup(). If this is not done the indicators vector grows indefinitely as it caches the deleted and NULLed indicators. See issue #1058 for more explanation/comments.