Hi,
On 2021-04-22 15:09:59 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > On 2021-04-22 14:37:26 -0400, Tom Lane wrote:
> >> OK, I could work with that. Shall we spell the error message the
> >> same as if it really were a virtual slot, or does it need to be
> >> different to avoid confusion?
>
> > Hm. Seems like it'd be better to have a distinct error message? Feels
> > like it'll often indicate separate issues whether a non-materialized
> > heap slot or a virtual slot has its system columns accessed.
>
> After thinking about it for a bit, I'm inclined to promote this to
> a user-facing error, and have all the slot types report
>
> ereport(ERROR,
> (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> errmsg("cannot retrieve a system column in this context")));
WFM.
> which is at least somewhat intelligible to end users. A developer
> trying to figure out why it happened would resort to \errverbose or
> more likely gdb in any case, so the lack of specificity doesn't
> seem like a problem.
I'm wondering if it'd be a good idea to add TupleTableSlotOps.name,
which we then could include in error messages without needing to end up
with per-slot-type code... But that's probably a separate project from
adding the error above.
> > Not entirely clear to me how we'd track whether we have valid system
> > column data or not once materialized - which I think is why we
> > historically had the cases where we returned bogus values.
>
> Right, but with this fix we won't need to materialize.
In this path - but it does seem mildly bothersome that we might do the
wrong thing in other paths. If we at least returned something halfway
sensible (e.g. NULL) instead of 128... I guess we could track whether
the tuple originated externally, or whether it's from a materialized
virtual tuple...
Greetings,
Andres Freund
