On 11/01/15 08:56, Kohei KaiGai wrote:
2015-01-11 10:40 GMT+09:00 Jim Nasby <jim.na...@bluetreble.com>:

Yeah there are actually several places in the code where "relid" means
index in range table and not oid of relation, it still manages to
confuse
me. Nothing this patch can do about that.

Well, since it's confused 3 of us now... should we change it (as a
separate patch)? I'm willing to do that work but don't want to waste
time if
it'll just be rejected.

Any other examples of this I should fix too?

Sorry, to clarify... any other items besides Scan.scanrelid that I should
fix?

This naming is a little bit confusing, however, I don't think it "should"
be
changed because this structure has been used for a long time, so reworking
will prevent back-patching when we find bugs around "scanrelid".

We can still backpatch; it just requires more work. And how many bugs do we
actually expect to find around this anyway?

If folks think this just isn't worth fixing fine, but I find the
backpatching argument rather dubious.

Even though here is no problem around Scan structure itself, a bugfix may
use the variable name of "scanrelid" to fix it. If we renamed it on v9.5, we
also need a little adjustment to apply this bugfix on prior versions.
It seems to me a waste of time for committers.


I tend to agree, especially as there is multiple places in code this would affect - RelOptInfo and RestrictInfo have same issue, etc.

--
 Petr Jelinek                  http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to