You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@age.apache.org by GitBox <gi...@apache.org> on 2022/11/26 01:12:31 UTC

[GitHub] [age] TropicalPenguin opened a new issue, #369: Crash on Edge MERGE after Edge MATCH

TropicalPenguin opened a new issue, #369:
URL: https://github.com/apache/age/issues/369

   **Describe the bug**
   Usually, the MERGE operation works. But when a merge operation follows a MATCH clause that includes an edge, the server terminates.
   
   **How are you accessing AGE (Command line, driver, etc.)?**
   - Command line
   - Golang driver
   
   **What data setup do we need to do?**
   ```pgsql
   ...
   SELECT create_graph('test');
   SELECT * from cypher('test', $$ CREATE (a {x:1})-[:foo]->(b {x:2}),(c {x:3}) $$) as (v0 agtype);
   ...
   ```
   
   **What is the necessary configuration info needed?**
   - N/A
   
   **What is the command that caused the error?**
   ```pgsql
   SELECT * from cypher('test', $$ MATCH ()-[a:foo]->(), (b {x:2}),(c {x:3}) MERGE (b)-[d:bar]->(c) RETURN d $$) as (v0 agtype);
   ```
   Error:
   ```
   server closed the connection unexpectedly
   	This probably means the server terminated abnormally
   	before or while processing the request.
   The connection to the server was lost. Attempting reset: Failed.
   ```
   
   **Expected behavior**
   The relationship should be created if it doesn't exist.
   
   **Environment (please complete the following information):**
   - Version: 1.1.0, commit ed44f91cb2607605ec2e225248e9ca46e146c215
   
   **Additional context**
   N/A
   


-- 
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: dev-unsubscribe@age.apache.org.apache.org

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


[GitHub] [age] parvit commented on issue #369: Crash on Edge MERGE after Edge MATCH

Posted by GitBox <gi...@apache.org>.
parvit commented on issue #369:
URL: https://github.com/apache/age/issues/369#issuecomment-1340065666

   I don't take issue with your excpetion, but understand that your burden of proof is pretty vague, what exactly would satisfy your requirement?
   
   * A different test case for the issue?
   * A generic explanation of the problem?
   * An indication of the code path that the issue triggers?
   * Something else?
   
   If you can't point to what objectively would meet your request, i would be shooting in the dark trying to meet your personal standard.
   
   This explanation would be really helpful also for future new contributors, it could be worthwhile to include it in the new tickets template / readme.
   


-- 
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: dev-unsubscribe@age.apache.org

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


[GitHub] [age] TropicalPenguin closed issue #369: Crash on Edge MERGE after Edge MATCH

Posted by GitBox <gi...@apache.org>.
TropicalPenguin closed issue #369: Crash on Edge MERGE after Edge MATCH
URL: https://github.com/apache/age/issues/369


-- 
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: dev-unsubscribe@age.apache.org

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


[GitHub] [age] jrgemignani commented on issue #369: Crash on Edge MERGE after Edge MATCH

Posted by GitBox <gi...@apache.org>.
jrgemignani commented on issue #369:
URL: https://github.com/apache/age/issues/369#issuecomment-1334476919

   I'm not sure what you mean,... it is already labeled as a bug.


-- 
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: dev-unsubscribe@age.apache.org

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


[GitHub] [age] TropicalPenguin commented on issue #369: Crash on Edge MERGE after Edge MATCH

Posted by GitBox <gi...@apache.org>.
TropicalPenguin commented on issue #369:
URL: https://github.com/apache/age/issues/369#issuecomment-1335930728

   I see; thanks for the clarification.


-- 
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: dev-unsubscribe@age.apache.org

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


[GitHub] [age] TropicalPenguin commented on issue #369: Crash on Edge MERGE after Edge MATCH

Posted by GitBox <gi...@apache.org>.
TropicalPenguin commented on issue #369:
URL: https://github.com/apache/age/issues/369#issuecomment-1336115272

   Thanks for raising it for consideration. This kind of crowd patronage of crafts seems kinda ideal for open source projects. More efficient than freelancers, since even if one can find a good developer they likely don't come with familiarity, either with the backend itself or a project's coding standards.


-- 
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: dev-unsubscribe@age.apache.org

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


[GitHub] [age] jrgemignani commented on issue #369: Crash on Edge MERGE after Edge MATCH

Posted by GitBox <gi...@apache.org>.
jrgemignani commented on issue #369:
URL: https://github.com/apache/age/issues/369#issuecomment-1340246767

   > - A generic explanation of the problem?
   
   For this specific case -
   
   What is the reason for the NULL? Is it just an overlooked case that is causing it? If so, please explain. Is it expected to sometimes be NULL, but it was just not properly dealt with? If so, please explain. Or, is there a more serious issue that was overlooked and needs to be addressed as well? If so, please explain.
   
   > - An indication of the code path that the issue triggers?
   
   Basically, I need to know that this solution fixes the issue and doesn't just mask it or introduce another. That is the heart of it.
   
   Hopefully, this is helpful.
   
   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: dev-unsubscribe@age.apache.org

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


[GitHub] [age] jrgemignani commented on issue #369: Crash on Edge MERGE after Edge MATCH

Posted by GitBox <gi...@apache.org>.
jrgemignani commented on issue #369:
URL: https://github.com/apache/age/issues/369#issuecomment-1341443449

   I have approve the workflow run - not to be confused with the actually merge. After that is done, I will make a decision on the merge. 
   
   There is still a **pending** coding standard update that is required.
   


-- 
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: dev-unsubscribe@age.apache.org

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


[GitHub] [age] parvit commented on issue #369: Crash on Edge MERGE after Edge MATCH

Posted by GitBox <gi...@apache.org>.
parvit commented on issue #369:
URL: https://github.com/apache/age/issues/369#issuecomment-1342158220

   > Closing with the merged PR; thanks for your work @parvit (invite is open for #343, in case the appetite's been whetted 😄), and for the guidance @jrgemignani.
   
   Of course, i was already working on it 😄 


-- 
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: dev-unsubscribe@age.apache.org

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


[GitHub] [age] jrgemignani commented on issue #369: Crash on Edge MERGE after Edge MATCH

Posted by GitBox <gi...@apache.org>.
jrgemignani commented on issue #369:
URL: https://github.com/apache/age/issues/369#issuecomment-1341511955

   Please don't take offense to any bolding, I did it strictly for emphasis. This is a compilation from many sources, not just from here. 
   
   **At the risk of repeating myself, and for future contributors and contributions -**
   
   Bounties have **NO** connection to Apache AGE. Meaning, we neither condone, prohibit, support monetarily or otherwise, bounties, in any way. This **may** change in the future. So, use/do them at your own risk.
   
   If someone wants to set a bounty, Apache AGE has **zero** obligation to support or approve any PR derived from it. If someone hopes that their PR will get approved, they need to, at the very least, follow our standards. Remember, Apache AGE did not ask for any bounties, nor put them up, nor fund them.
   
   Here is a rough guide of minimum expectations for **any** PR -
   
   1. All bug fixes must have explanations as to why/how they address the problem. Meaning, what was the root cause of the bug and why (how) does this PR correct it. As I have stated above, think of it like a proof. You need to prove to Apache AGE PMC members and committers that your work resolves the issue, not masks it, nor generates potentially hidden issues.
   2. All contributions, without exception, must conform to our coding standards.
   3. Adequate commenting in the code.
   4. Adequate documentation in the PR commit log entry.
   
   Please understand, we will not always be prompt in responding to these requests. Apache AGE PMC and committers have a lot of work to do. However, we will try our best.
   
   Hopefully, this will be helpful going forward.
   
   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: dev-unsubscribe@age.apache.org

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


[GitHub] [age] jrgemignani commented on issue #369: Crash on Edge MERGE after Edge MATCH

Posted by GitBox <gi...@apache.org>.
jrgemignani commented on issue #369:
URL: https://github.com/apache/age/issues/369#issuecomment-1341580680

   @TropicalPenguin I think that once @parvit submits the above mentioned changes, we will be done with this issue :)


-- 
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: dev-unsubscribe@age.apache.org

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


[GitHub] [age] TropicalPenguin commented on issue #369: Crash on Edge MERGE after Edge MATCH

Posted by GitBox <gi...@apache.org>.
TropicalPenguin commented on issue #369:
URL: https://github.com/apache/age/issues/369#issuecomment-1328603659

   Bounty opened: https://app.bountysource.com/issues/114119874-crash-on-edge-merge-after-edge-match


-- 
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: dev-unsubscribe@age.apache.org

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


[GitHub] [age] parvit commented on issue #369: Crash on Edge MERGE after Edge MATCH

Posted by GitBox <gi...@apache.org>.
parvit commented on issue #369:
URL: https://github.com/apache/age/issues/369#issuecomment-1341628881

   Sorry but i don't seem to be able to find any comment or review on the PR, could you check? thanks.


-- 
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: dev-unsubscribe@age.apache.org

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


[GitHub] [age] TropicalPenguin commented on issue #369: Crash on Edge MERGE after Edge MATCH

Posted by GitBox <gi...@apache.org>.
TropicalPenguin commented on issue #369:
URL: https://github.com/apache/age/issues/369#issuecomment-1342028865

   Closing with the merged PR; thanks for your work @parvit (invite is open for https://github.com/apache/age/issues/343, in case the appetite's been whetted :smile:), and for the guidance @jrgemignani.


-- 
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: dev-unsubscribe@age.apache.org

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


[GitHub] [age] jrgemignani commented on issue #369: Crash on Edge MERGE after Edge MATCH

Posted by GitBox <gi...@apache.org>.
jrgemignani commented on issue #369:
URL: https://github.com/apache/age/issues/369#issuecomment-1339946991

   BTW, I've added those specific questions to that ticket. We need to verify that the work that was done and is proposed takes those issues into account.
   


-- 
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: dev-unsubscribe@age.apache.org

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


[GitHub] [age] parvit commented on issue #369: Crash on Edge MERGE after Edge MATCH

Posted by GitBox <gi...@apache.org>.
parvit commented on issue #369:
URL: https://github.com/apache/age/issues/369#issuecomment-1341581471

   I'm not sure what coding standard issue you're referring to, can you elaborate please?


-- 
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: dev-unsubscribe@age.apache.org

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


[GitHub] [age] jrgemignani commented on issue #369: Crash on Edge MERGE after Edge MATCH

Posted by GitBox <gi...@apache.org>.
jrgemignani commented on issue #369:
URL: https://github.com/apache/age/issues/369#issuecomment-1341804959

   Looks good. Waiting on review (just double checking) from @dehowef who will be on this evening. Waiting on Travis CI to do a build check.
   
   Tomorrow, provided Travis CI is successful and there are no other issues, one of us will merge it.
   
   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: dev-unsubscribe@age.apache.org

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


[GitHub] [age] jrgemignani commented on issue #369: Crash on Edge MERGE after Edge MATCH

Posted by GitBox <gi...@apache.org>.
jrgemignani commented on issue #369:
URL: https://github.com/apache/age/issues/369#issuecomment-1335967903

   @TropicalPenguin We have started a discussion on the issues of **bounties**.
   
   Btw, I want to thank you for being an active member of the AGE community and believing in AGE enough to put up bounties for issues that you hold as important.


-- 
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: dev-unsubscribe@age.apache.org

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


[GitHub] [age] parvit commented on issue #369: Crash on Edge MERGE after Edge MATCH

Posted by GitBox <gi...@apache.org>.
parvit commented on issue #369:
URL: https://github.com/apache/age/issues/369#issuecomment-1339858275

   Hi, please review my PR #376 for this change


-- 
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: dev-unsubscribe@age.apache.org

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


[GitHub] [age] TropicalPenguin commented on issue #369: Crash on Edge MERGE after Edge MATCH

Posted by GitBox <gi...@apache.org>.
TropicalPenguin commented on issue #369:
URL: https://github.com/apache/age/issues/369#issuecomment-1340416894

   For what it's worth, I've checked out the PR locally and run some basic tests; seems promising so far. I'll stress test more soon, as I need to figure out whether some bugs I'm seeing in behavior related to this are from my own code or AGE :sweat_smile: 


-- 
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: dev-unsubscribe@age.apache.org

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


[GitHub] [age] TropicalPenguin commented on issue #369: Crash on Edge MERGE after Edge MATCH

Posted by GitBox <gi...@apache.org>.
TropicalPenguin commented on issue #369:
URL: https://github.com/apache/age/issues/369#issuecomment-1331658798

   The query here was simplified to highlight what seems to be the part of a larger expression that's causing problems for me. In that larger query, there are additional SET clauses on the matched edge, in addition to the merge. Hopefully once this is fixed, I suspect the larger query will just work.


-- 
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: dev-unsubscribe@age.apache.org

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


[GitHub] [age] parvit commented on issue #369: Crash on Edge MERGE after Edge MATCH

Posted by GitBox <gi...@apache.org>.
parvit commented on issue #369:
URL: https://github.com/apache/age/issues/369#issuecomment-1340467019

   @jrgemignani So, with respect to the issue_369 test script present in patch, if i change:
   ``` ()-[a:foo]->()  ```
   to either:
   ``` (x)-[a:foo]->()  ```
   or
   ``` ()-[a:foo]->(y)  ```
   
   The extension still segfaults, if instead i use:
   ``` (x)-[a:foo]->(y)  ```
   works and outputs like with the patch applied.
   
   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.
   
   Please advise.
   


-- 
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: dev-unsubscribe@age.apache.org

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


[GitHub] [age] TropicalPenguin commented on issue #369: Crash on Edge MERGE after Edge MATCH

Posted by GitBox <gi...@apache.org>.
TropicalPenguin commented on issue #369:
URL: https://github.com/apache/age/issues/369#issuecomment-1341548423

   No offense taken on my part. As far as I know there's no requirement that a PR be merged in order for me to fulfill the obligation I create for myself when I pledge to a bounty. It may, however, require closing - at least temporarily - an issue before the corresponding PR is merged. There's no particular urgency for me to use code from the main branch, however convenient I imagine it being there to the community at large.


-- 
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: dev-unsubscribe@age.apache.org

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


[GitHub] [age] TropicalPenguin commented on issue #369: Crash on Edge MERGE after Edge MATCH

Posted by GitBox <gi...@apache.org>.
TropicalPenguin commented on issue #369:
URL: https://github.com/apache/age/issues/369#issuecomment-1341426242

   Given that anonymous entities are kosher, it seems intuitive to me that the resulting entities with a NULL name in find_transform_entity isn't an error condition, and can safely be skipped during a search, as is achieved by @parvit's patch.
   
   Not noticing any regressions in my local testing, and the behavior in my use-cases that were failing, are now functionally correct.


-- 
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: dev-unsubscribe@age.apache.org

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


[GitHub] [age] jrgemignani commented on issue #369: Crash on Edge MERGE after Edge MATCH

Posted by GitBox <gi...@apache.org>.
jrgemignani commented on issue #369:
URL: https://github.com/apache/age/issues/369#issuecomment-1331556236

   Just curious, why are you matching something that is extraneous - **()-[a:foo]->()** to the merge? Because, if you remove that part, the match works correctly.
   
   ```
   psql-11.5-5432-pgsql=# SELECT * from cypher('test', $$ MATCH (b {x:2}),(c {x:3}) MERGE (b)-[d:bar]->(c) RETURN d $$) as (v0 agtype);
                                                               v0
   --------------------------------------------------------------------------------------------------------------------------
    {"id": 9570149208162305, "label": "bar", "end_id": 281474976710659, "start_id": 281474976710658, "properties": {}}::edge
   (1 row)
   
   psql-11.5-5432-pgsql=#
   ```
   
   That aside, there appears to be an edge case that was overlooked with the combination of the two. That needs to be addressed and resolved.


-- 
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: dev-unsubscribe@age.apache.org

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


[GitHub] [age] TropicalPenguin commented on issue #369: Crash on Edge MERGE after Edge MATCH

Posted by GitBox <gi...@apache.org>.
TropicalPenguin commented on issue #369:
URL: https://github.com/apache/age/issues/369#issuecomment-1334767611

   Oh, apologies. I'll try to be more clear. I realised that a monetary incentive/bounty for a contributor to find a fix isn't much of an incentive if no-one knows about it. So I thought another label (such as 'bounty'), added alongside the existing 'bug' label, would be helpful.  


-- 
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: dev-unsubscribe@age.apache.org

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


[GitHub] [age] jrgemignani commented on issue #369: Crash on Edge MERGE after Edge MATCH

Posted by GitBox <gi...@apache.org>.
jrgemignani commented on issue #369:
URL: https://github.com/apache/age/issues/369#issuecomment-1340027047

   I get that. But, if one cannot answer those questions, one cannot say they have actually fixed the issue. Think of it like a proof. Show me that your fix is the correct solution. If you can show me, I'll be more than happy to approve it. I know that your fix will eliminate the NULL pointer error. But, why is it NULL in the first place?
   
   Btw, all of our team members are expected, without exception, to be able to answer questions like those about an issue. I cannot, in good faith, approve something from someone else - especially from outside - who cannot. That would set a bad precedent.
   
   Please don't take this personally, this applies to everyone, including myself.
   
   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: dev-unsubscribe@age.apache.org

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


[GitHub] [age] jrgemignani commented on issue #369: Crash on Edge MERGE after Edge MATCH

Posted by GitBox <gi...@apache.org>.
jrgemignani commented on issue #369:
URL: https://github.com/apache/age/issues/369#issuecomment-1339944375

   @TropicalPenguin I feel the need to point out something, with regards to others submitting patches, and the problems they can cause. Especially, if there is a bounty on the bug.
   
   This bug, for example, is a NULL pointer error - which I had already found out a while back through gdb. While I could have submitted a patch to cover the NULL pointer, I didn't because these questions still remain - 
   
   - Why is the pointer NULL? 
   - Was it supposed to be something else and that something needs fixing?
   - Was it just a case that was overlooked?
   - Is there a flaw in the processing that leaves it as NULL?
   
   These are points that whomever does the patch needs to answer.


-- 
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: dev-unsubscribe@age.apache.org

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


[GitHub] [age] parvit commented on issue #369: Crash on Edge MERGE after Edge MATCH

Posted by GitBox <gi...@apache.org>.
parvit commented on issue #369:
URL: https://github.com/apache/age/issues/369#issuecomment-1340002447

   @jrgemignani I understand your point of view but the request in the ticket is very specific and a patch that maybe changes a lot more things would be unlikely to be accepted in my experience.
   


-- 
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: dev-unsubscribe@age.apache.org

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


[GitHub] [age] jrgemignani commented on issue #369: Crash on Edge MERGE after Edge MATCH

Posted by GitBox <gi...@apache.org>.
jrgemignani commented on issue #369:
URL: https://github.com/apache/age/issues/369#issuecomment-1332571650

   @TropicalPenguin MERGE & SET have generally been problematic to implement, especially when used in conjunction with other commands. There are a lot of reasons why this is the case.
   
   As both are update commands, they are generally tricky to implement properly - managing transaction ids, command ids, etc. Additionally, both commands have to integrate correctly into the transaction system which works differently depending on the scope of the transaction. 
   
   And, we have to find all of the different ways a developer, **you**, will put these pieces together. So, thank you for your input!
   
   We will need to look into what is causing an issue here.


-- 
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: dev-unsubscribe@age.apache.org

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


[GitHub] [age] TropicalPenguin commented on issue #369: Crash on Edge MERGE after Edge MATCH

Posted by GitBox <gi...@apache.org>.
TropicalPenguin commented on issue #369:
URL: https://github.com/apache/age/issues/369#issuecomment-1335850100

   The incentive is already created/escrow'd at the BountySource link posted earlier...was only hoping to highlight that, not expecting anyone else to chip in. Oh well.


-- 
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: dev-unsubscribe@age.apache.org

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


[GitHub] [age] jrgemignani commented on issue #369: Crash on Edge MERGE after Edge MATCH

Posted by GitBox <gi...@apache.org>.
jrgemignani commented on issue #369:
URL: https://github.com/apache/age/issues/369#issuecomment-1341591341

   Please review your PR, I put it there.


-- 
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: dev-unsubscribe@age.apache.org

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


[GitHub] [age] jrgemignani commented on issue #369: Crash on Edge MERGE after Edge MATCH

Posted by GitBox <gi...@apache.org>.
jrgemignani commented on issue #369:
URL: https://github.com/apache/age/issues/369#issuecomment-1339933249

   > Hi, please review my PR #376 for this issue
   
   Please review my comments on the PR.
   
   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: dev-unsubscribe@age.apache.org

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


[GitHub] [age] jrgemignani commented on issue #369: Crash on Edge MERGE after Edge MATCH

Posted by GitBox <gi...@apache.org>.
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: dev-unsubscribe@age.apache.org

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


[GitHub] [age] jrgemignani commented on issue #369: Crash on Edge MERGE after Edge MATCH

Posted by GitBox <gi...@apache.org>.
jrgemignani commented on issue #369:
URL: https://github.com/apache/age/issues/369#issuecomment-1335839250

   Sorry, I have zero authority, nor the ability, to create a **monetary** incentive/bounty for a bug. 


-- 
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: dev-unsubscribe@age.apache.org

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


[GitHub] [age] jrgemignani commented on issue #369: Crash on Edge MERGE after Edge MATCH

Posted by GitBox <gi...@apache.org>.
jrgemignani commented on issue #369:
URL: https://github.com/apache/age/issues/369#issuecomment-1335918351

   Again, the decision to support or highlight external posts like bounties isn't up to me. I can bring this up in our meeting next week, though.


-- 
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: dev-unsubscribe@age.apache.org

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


[GitHub] [age] TropicalPenguin commented on issue #369: Crash on Edge MERGE after Edge MATCH

Posted by GitBox <gi...@apache.org>.
TropicalPenguin commented on issue #369:
URL: https://github.com/apache/age/issues/369#issuecomment-1334094879

   > Bounty opened: https://app.bountysource.com/issues/114119874-crash-on-edge-merge-after-edge-match
   
   @jrgemignani Any chance this issue could get a label to indicate it has one?


-- 
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: dev-unsubscribe@age.apache.org

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