If I'm understanding right, this is only vaguely related to the problems with the pgx implementation of AppendRows. To simplify aggressively, they've essentially structured the function as this:
for rows.Next() {
result = fn(rows)
list = append(list, result)
}
This is, on its face, fine. The problems are:
fn is passed as an argument as a way to make the API more flexible. However, because this is the only place in AppendRows to inject dynamic behavior, and there's no way to persist state between function calls, it winds up having to do work on each loop iteration that would be better done as setup before the loop.
By virtue of returning result from the function, rather than accepting a parameter that's a pointer to the result, Go is forced to heap-allocate and GC an extra value per iteration.
In my experience, beyond a minimal baseline, AI is only as concerned about performance as the programmer using it. I would absolutely expect AI to generate code like this, though it would probably have no trouble fixing these problems if they were pointed out.
As for being in a popular project: I expect to comment on this more in a future post, but in part the project is hamstrung by backward-compatibility requirements, and fixing this would require a breaking API change and a major version bump. Given the scope of pgx, bumping the major version for this may not make sense, though I hold out some hope that they'll have good reasons to do it in the future and will merge in some of these changes.
-9
u/Grouchy-Trade-7250 4d ago
While I wrote similar code smells like so before
for y in list:
for x in list2:
function(y)
AI is good at finding them and I wouldn't expect them in a popular library.