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 по дате отправления: