Robert Haas <robertmh...@gmail.com> writes:
> It's quite possible that this patch isn't completely correct and it's
> also possible that I'm missing some deeper problem with this idea that
> just isn't exercised by the regression tests. But overall I find these
> results fairly encouraging -- it just wasn't very hard to make the
> regression tests pass.
My guess is that there are more places that assume eref->aliasname
isn't NULL than you've caught here; and some of them probably are in
code we don't control. And you didn't even update the comments for
struct Alias.
Is there some strong reason to insist on making that core-dump-risking
change, rather than simply assigning the now-one-size-fits-all alias
when creating Alias nodes?
> I did a bit of historical checking at the comment that eref->aliasname
> is required to be present was added in commit
> 3a94e789f5c9537d804210be3cb26f7fb08e3b9e, Tom Lane, 2000. I can't
> immediately tell what the reasoning was. Copying Tom in case he has
> thoughts.
Well, that was 24 years ago ... but the commit message seems to only
be about the requirement that user-written sub-SELECT-in-FROM have
an alias. Which is a requirement we've since dropped.
When it comes to system-supplied aliases, I'd argue that the tradeoff
is about the same: forcing somebody or something that has a clue what
the relation is to define an alias, versus falling back to some generic
alias. If we're okay with generic aliases for user-written sub-SELECT
then it's not much of a step to generic aliases for system-defined
relations. I won't argue hard about that --- certainly things like
"*VALUE*" aren't very pretty. But I'd really rather not switch to
"eref->aliasname can be NULL": that's introducing a coding gotcha for
zero benefit that I can detect.
regards, tom lane
