Fix comments for ChangeVarNodes() and related functions

Поиск
Список
Период
Сортировка
От Richard Guo
Тема Fix comments for ChangeVarNodes() and related functions
Дата
Msg-id CAMbWs480j16HC1JtjKCgj5WshivT8ZJYkOfTyZAM0POjFomJkg@mail.gmail.com
обсуждение исходный текст
Ответы Re: Fix comments for ChangeVarNodes() and related functions
Список pgsql-hackers
I happend to notice this comment for ChangeVarNodes():

 * Specifying 'change_RangeTblRef' to false allows skipping RangeTblRef.
 * See ChangeVarNodesExtended for details.

However, I couldn't find "change_RangeTblRef" anywhere in the code
base.  I suspect this is a leftover from development.  In any case, I
think we should fix it.

Also, the comment for ChangeVarNodesExtended() contains an extra
space.

 * ChangeVarNodesExtended - similar to ChangeVarNodes, but with an  additional

In addition, the comment for replace_relid_callback() has an odd line
wrap.

 * SJE needs to skip the RangeTblRef node
 * type.  During SJE's last step, remove_rel_from_joinlist() removes

And one comment within replace_relid_callback() says:

 * as "t1.a" is not null.  We use qual() to check for such a case,

I believe it should be "equal" rather than "qual".

Attached is a proposed patch with the fixes.  It also includes some
sentence revisions for smoother wording.

FWIW, I think the comment inside remove_rel_from_query() is also a
mess.  For example, one comment in this function states:

 * Finally, we must recompute per-Var attr_needed and per-PlaceHolderVar
 * ph_needed relid sets.

However, there is no corresponding code in this function that
recomputes the attr_needed or ph_needed bits; that recomputation
happens elsewhere.

Another comment in this function states:

 * fail to remove other now-removable outer joins.  And our removal of the
 * join clause(s) for this outer join may mean that Vars that were
 * formerly needed no longer are.

However, this function is now used not only to remove outer joins but
also inner (self) joins.  So the phrase "for this outer join" in the
comment is misleading.

However, it seems to me that it would be better to address the
comments for remove_rel_from_query() in a separate patch.

- Richard

Вложения

В списке pgsql-hackers по дате отправления: