jrgemignani commented on issue #369: URL: https://github.com/apache/age/issues/369#issuecomment-1341576118
@parvit > Obviously the null check is valid by itself but the question would > now be: are anonymous nodes allowed or should they be blocked? > > If they are allowed than probably they should have an auto generated name attached. > If they are not then there should be a syntax error triggered before evaluation. Yeah, I pulled the code into the debugger and verified the issue and cause as well. It appears that the list of entities can have members without actual names i.e. NULL; I believe that is okay. Still, the original code does nothing to check for the possibility that those NULL entries may exist - with a simple check for NULL - to skip over them and avoid a crash. Nor were there any regression tests that checked for that possibility. So, thank you for adding one. The only question is, should the function return NULL if name is NULL? I think it is appropriate as you can't guarantee that your NULL is the correct NULL in the list. As for the **anonymous** nodes allowed or blocked,... I do think that this specific usage was extraneous as it wasn't linked directly to anything else. I would ask that you - - Add regression tests for all of the cases that you mentioned above - Fix the coding standard issue. Then I can give it a final review and hopefully put it behind us. john -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
