I think I'm finally ready for a review again, so I'll update the commitfest app.
I have reviewed this on code level.
1. Patch gets applied cleanly. 2. make/make install/make check all are fine
No issues found till now.
However some cosmetic points:
1. * The API of this function is identical to convert_ANY_sublink_to_join's, * except that we also support the case where the caller has found NOT EXISTS, * so we need an additional input parameter "under_not".
Since now, we do support NOT IN handling in convert_ANY_sublink_to_join() we do have "under_not" parameter there too. So above comments near convert_EXISTS_sublink_to_join() function is NO more valid.
2. Following is the unnecessary change. Can be removed:
a. + * queryTargetListListCanHaveNulls ... +queryTargetListCanHaveNulls(Query *query)
Function name is queryTargetListCanHaveNulls() not queryTargetListListCanHaveNulls()
b. * For example the presense of; col IS NOT NULL, or col = 42 would allow
presense => presence
4. get_attnotnull() throws an error for invalid relid/attnum. But I see other get_att* functions which do not throw an error rather returning some invalid value, eg. NULL in case of attname, InvalidOid in case of atttype and InvalidAttrNumber in case of attnum. I have observed that we cannot return such invalid value for type boolean and I guess that's the reason you are throwing an error. But somehow looks unusual. You had put a note there which is good. But yes, caller of this function should be careful enough and should not assume its working like other get_att*() functions. However for this patch, I would simply accept this solution. But I wonder if someone thinks other way round.
Testing more on SQL level.
However, assigning it to author to think on above cosmetic issues.
Thanks
--
Jeevan B Chalke Principal Software Engineer, Product Development EnterpriseDB Corporation The Enterprise PostgreSQL Company