vBulletin Search Engine Optimization
| |||||||
| Register | FAQ | Members List | Calendar | Search | Today's Posts | Mark Forums Read |
| ||||
| Per EDB's Coverity analysis, "runtimeKeyInfo" is only non-NULL if "econtext" is also non-NULL, so we can eliminate a conditional on the former by moving it inside an "if" block for the latter. Per discussion of earlier similar changes, this is not for performance reasons but for code clarity. Barring any objections I'll apply this tonight or tomorrow. -Neil ---------------------------(end of broadcast)--------------------------- TIP 5: don't forget to increase your free space map settings |
| ||||
| Neil Conway <neilc@samurai.com> writes: > Per EDB's Coverity analysis, "runtimeKeyInfo" is only non-NULL if > "econtext" is also non-NULL, so we can eliminate a conditional on the > former by moving it inside an "if" block for the latter. This is premature optimization par excellance --- it saves one if-test, in a not very performance-critical routine. I disagree that it improves the code clarity: it's far from obvious that being-passed-an-outer-tuple is the same condition as has-runtime-keys, and I can think of numerous possible future changes that would render the conditions distinct again. So please, don't do it. > ! if (!(IsA(leftop, Var) && > ! var_is_rel((Var *) leftop))) > elog(ERROR, "indexqual doesn't have key on left side"); > ! if (!(IsA(leftop, Var) && var_is_rel((Var *) leftop))) > elog(ERROR, "indexqual doesn't have key on left side"); Just FYI, the reason for the line break is that pgindent will make it look uglier if it's all on one line. The IsA construct seems to confuse pgindent somehow ... regards, tom lane ---------------------------(end of broadcast)--------------------------- TIP 5: don't forget to increase your free space map settings |