[GitHub] [age] moeed-k commented on pull request #586: Optimize age_exists function

["moeed-k (via GitHub)" <gi...@apache.org>]

moeed-k commented on PR #586:
URL: https://github.com/apache/age/pull/586#issuecomment-1400908540

   > @moeed-k I appreciate that you looked into optimizing this function. However, can you please thoroughly explain why the code that you want to remove, can actually be removed.
   
   Yes of course. According to my understanding, all properties in AGE can either have a NULL value, or *some* actual value, and the exists(property) function only serves to check whether this value is NULL (returns false) or not (returns true). I was testing around the code in the debugger and I realized that whenever the property of a function is NULL (e.g. prop1: null), it is always filtered out by line number 5464 ( PG_ARGISNULL(0) ). The macro returns true, and the function returns false in the next line.
   
   The next few lines involve getting the agtype root node, checking if it is a scalar, getting the value of the root if it _is_ a scalar, and then finally accessing the value type to check if it is NULL or not. However, since all NULL values were already filtered out at line 5464, this last check is never actually true (shown below).
   ```
           if (agtv_value->type == AGTV_NULL)
               PG_RETURN_BOOL(false);
   ```
   In all my testing, I was unable to make this condition true. What this does mean, however, is that whenever there is a non-null scalar value passed to exists(), it goes through the trouble of making all those redundant checks to verify the scalar's value type. 
   
   The regression tests didn't flag any errors, and my own testing was also yielding the same output as before. 
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@age.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org