Tom Lane said:
> If we did it with a WithOrdinality expression node, the result would
> always be of type RECORD, and we'd have to use blessed tuple
> descriptors to keep track of exactly which record type a particular
> instance emits. This is certainly do-able (see RowExpr for
> precedent).
Maybe RowExpr is a precedent for something, but it has this
long-standing problem that makes it very hard to use usefully:
postgres=# select (r).* from (select row(a,b) as r from (values (1,2)) v(a,b)) s;
ERROR: record type has not been registered
> It seems way too short on comments. [...]
This can certainly be addressed.
> but it sure looks like it flat out removed several existing
> regression-test cases
Here's why, in rangefuncs.sql:
--invokes ExecReScanFunctionScan
SELECT * FROM foorescan f WHERE f.fooid IN (SELECT fooid FROM foorescan(5002,5004)) ORDER BY 1,2;
I don't think that has invoked ExecReScanFunctionScan since 7.4 or so.
It certainly does not do so now (confirmed by gdb as well as by the
query plan). By all means keep the old tests if you want a
never-remove-tests-for-any-reason policy, but having added tests that
actually _do_ invoke ExecReScanFunctionScan, I figured the old ones
were redundant. (Also, these kinds of tests can be done a bit better
now with values and lateral rather than creating and dropping tables
just for the one test.)
> and a few existing comments as well.
I've double-checked, and I don't see any existing comments removed.
> FWIW, I concur with the gripe I remember seeing upthread that the
> default name of the added column ought not be "?column?".
This seems to be a common complaint, but gives rise to two questions:
1) what should the name be?
2) should users be depending on it?
I've yet to find another db that actually documents a specific column
name for the ordinality column; it's always taken for granted that the
user should always be supplying an alias. (Admittedly there are not
many dbs that support it at all; DB2 does, and I believe Teradata.)
--
Andrew (irc:RhodiumToad)
