Andres Freund <andres@anarazel.de> writes:
> - The !caseExpr->defresult result branch is currently unreachable (and
> its equivalent was before the patch) because transformCaseExpr()
> generates a default expression. I'm inclined to replace the dead code
> with an assertion. Any reason not to do that?
Ah-hah. I'd noted that that code wasn't being reached when I did some
code coverage checks this morning, but I hadn't found the cause yet.
Yeah, replacing the if-test with an assert seems fine.
> I think there's some argument to be made that we should move all of
> execSRF.c's logic to their respective callsites. There's not really
> much shared code: ExecEvalFuncArgs(), tupledesc_match(), init_sexpr() -
> and at least the latter would even become a bit simpler if we didn't
> support both callers.
Possibly. All that code could stand to be rethought, probably, now that
its mission is just to handle the SRF case. But at least all the cruft is
in one place for the moment. And I don't think it's anything we have to
fix before v10, it's just cosmetic.
> Thanks a lot for your work on the patch!
You're welcome!
regards, tom lane
