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

Added NULLIF() function #910

Closed
wants to merge 0 commits into from
Closed

Conversation

WendelLana
Copy link
Contributor

PR for issue #908.

Added NULLIF() function that takes 2 arguments of "any" type (converting non-AGTYPE types to AGTYPE values) and return NULL if the two specified expressions are equal, if not returns the first expression.

Added regression tests.

@dehowef dehowef requested a review from jrgemignani May 15, 2023 21:48
@jrgemignani
Copy link
Contributor

@WendelLana Is this PR necessary? There is a response in issue #908 stating how this could be done without this -

image

Additionally, if it is necessary, you need to bring your PR up-to-date with the master branch -

image

@dehowef dehowef self-requested a review June 23, 2023 21:37
@WendelLana
Copy link
Contributor Author

WendelLana commented Jun 24, 2023

@jrgemignani In my issue I've said using CASE would be an alternative to NULLIF too, but I find that the NULLIF() function is less verbose and simpler to use, even when nested. This is also part of a project (Conditional Statement Improvements) that you can find here. So, I believe this would be a good enhancement to Apache AGE, considering its usefulness as a function in Postgres.

@jrgemignani
Copy link
Contributor

jrgemignani commented Jun 27, 2023

@WendelLana Are AGDB-INC projects separate from AGE? Also, your build still fails. Edit: sorry, I didn't mean that to sound harsh.

@WendelLana
Copy link
Contributor Author

@jrgemignani I believe they are related, the description is about Apache AGE and there is even a link to the Apache AGE website in the readme of the repository. I apologize for the build issue, my text editor removed all the blank spaces at the end of the lines, which are necessary in the output file (expr.out) for the SQL.

@rafsun42
Copy link
Member

@WendelLana is the nullif function referred in the openCypher specification?

@WendelLana
Copy link
Contributor Author

@rafsun42 I don't believe so. However, there is an issue on the Neo4j repository about adding this function, and it's also easier to use and comprehend compared to a command like this:
COALESCE(CASE columnname WHEN '' THEN NULL ELSE columnname END, 'STRING')
or nested CASE clauses.

@rafsun42
Copy link
Member

@WendelLana Could you elaborate about the issue you are referring to?

Could you also give a use case of nullif function?

@WendelLana
Copy link
Contributor Author

I'm referring to this issue that shows one use case. I believe we could use it in some scenarios, for instance, when dividing by zero. Consider the following Cypher query:

MATCH (n)-[]->(m)
RETURN n.value / nullif (m.value, 0)

Additionally, I find this command:

RETURN NULLIF(n.value, m.value)

to be better and less verbose than:

RETURN 
    CASE n.value = m.value THEN NULL
    ELSE n.value

I know it may seem like a very specific case, but I genuinely think having a function like Postgres' NULLIF is beneficial as it promotes more integration between SQL and Cypher. This permits a simpler logic when writing a Cypher query using SQL-like logic.

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

Successfully merging this pull request may close these issues.

3 participants