Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implemented age_tail function #1013

Closed
wants to merge 3 commits into from

Conversation

MatheusFarias03
Copy link
Contributor

Added the age_tail() function, which returns a list containing all the elements, excluding the first one, from a list. It is essentially the same function that openCypher has: https://neo4j.com/docs/cypher-manual/current/functions/list/#functions-tail

Added the age_tail() function, which returns a list  containing all the elements, excluding the first one, from a list. It is essentially the same function that openCypher has: https://neo4j.com/docs/cypher-manual/current/functions/list/#functions-tail

-- should throw errors
SELECT * FROM cypher('for_tail', $$ RETURN tail(123) $$) AS (tail agtype);
ERROR: graph "for_tail" does not exist
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it the error we are expecting?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was not, because of the wrong name for the graph. Fixed with the new commit.

agis_result.res = push_agtype_value(&agis_result.parse_state,
WAGT_END_ARRAY, NULL);

PG_RETURN_POINTER(agtype_value_to_agtype(agis_result.res));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we free the agtype_value from the memory (i.e. using pfree_agtype_value), since we are not returning it?

Copy link
Contributor Author

@MatheusFarias03 MatheusFarias03 Oct 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe at the time I created this PR we did not have the pfree_agtype_value() function, at least not in utils/adt/agtype_util.c. I will execute the git pull command and then git rebase so that the branch is updated and then make the necessary modifications. I hope you don't mind me asking, but pfree_agtype_value() should be called with which argument?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think @rafsun42 might be referring to agis_result.res but that would mean assigning a variable the return value of agtype_value_to_agtype and returning that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ook! I did that and it seems to work fine, but I just had to do it on another branch because I ended up making a mess of a code after git rebase. Is it okay if I create another PR and close this one? Sorry for this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MatheusFarias03 If that is the only way, then you don't have an option.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's the updated PR with all the necessary fixes: #1272

src/backend/utils/adt/agtype.c Show resolved Hide resolved
src/backend/utils/adt/agtype.c Show resolved Hide resolved
src/backend/utils/adt/agtype.c Show resolved Hide resolved
@MatheusFarias03
Copy link
Contributor Author

I have added some curly braces in the if statements.

Copy link
Contributor

@jrgemignani jrgemignani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pending @rafsun42 question resolution.

@MatheusFarias03
Copy link
Contributor Author

Closing this PR - There's an updated PR that fixes everything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants