Mark Dilger <hornschnorter@gmail.com> writes:
>> The swtich in src/backend/parser/analyze.c circa line 2819 should
>> probably have an explicit case for RTE_RESULT rather than just a
>> comment and allowing the default to log "unrecognized RTE type",
>> since it's not really unrecognized, just unexpected. But I'm not
>> too exercised about that, and won't argue if you want to leave it
>> as is.
Meh --- I doubt we need two different "can't happen" messages there.
The reason I treated this differently from some other places is that
transformLockingClause is only used during parsing, when there certainly
shouldn't be any RTE_RESULT RTEs present. Some of the other functions
in that file are also called from outside the parser, so that it's
less certain they couldn't see a RTE_RESULT, so I added explicit
errors for them.
There's been some talk of having more uniform handling of switches
on enums, which might change the calculus here (i.e. we might not want
to have a default: case at all). But I don't feel a need to add code
to transformLockingClause till we have that.
>> Similarly, in src/backend/commands/explain.c, should there be a
>> case for T_Result in ExplainTargetRel's switch circa line 2974?
>> I'm not sure given your design whether this could ever be relevant,
>> but I noticed that T_Result / RTE_RELATION isn't handled here.
We don't get to that function for a T_Result plan (cf. switch in
ExplainNode), so it'd just be dead code.
> Ok, version 6 of the patch applies cleanly, compiles, and passes
> all tests for me on my platform (mac os x).
Again, thanks for reviewing!
Pushed now. I did yield to popular demand and change the temp table's
name in join.sql to be "onerow" instead of "dual" ;-)
regards, tom lane