Fix parameter binding for arrow results

[nbenn]

Ad #4219

[Mytherin]

@krlmlr could you perhaps have a look?

[hannes]

Why can't we just use res@query_result in all cases?

[krlmlr]

Or should we get rid of the "query_result" slot and use env$query_result consistently? It feels weird to have this general-purpose env sitting there, and then the super-strict "query_result" S4 slot.

I also wonder if we also/instead need to special-case for Arrow in dbBind__duckdb_result() .

I agree that we want to get that right independently of r-dbi/DBI#390.

[nbenn]

The proposed fix is not the most elegant, I agree. I went with more of a "quick fix" approach rather than changing the logic in multiple places. We might be changing some of this again when the DBI interface extension for arrow becomes available.

Why can't we just use res@query_result in all cases?

While dbBind() does return the result object, it does so invisibly and in doing so it might be called only for its side effect. Therefore I used the already available env slot for its reference semantics.

Or should we get rid of the "query_result" slot and use env$query_result consistently?

Sure, we could do that. This would entail a bit of a rewrite, which is why I did not go directly for this.

I also wonder if we also/instead need to special-case for Arrow in dbBind__duckdb_result()

@krlmlr could you elaborate? I don't understand what you mean exactly.

[krlmlr]

I took another look. It appears to me that we need to change too much here to make it work, and then we would be deprecating this functionality right away with the new DBI Arrow interface. What do you think?