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

Normalize as boolean deletes boolean properties, too. #3594

Open
michael-simons opened this issue May 24, 2023 · 2 comments
Open

Normalize as boolean deletes boolean properties, too. #3594

michael-simons opened this issue May 24, 2023 · 2 comments
Labels
core-functionality Adding new procedure, function or signature to APOC core docs

Comments

@michael-simons
Copy link
Contributor

michael-simons commented May 24, 2023

Expected Behavior (Mandatory)

Normalize to boolean should normalise string properties to boolean, not delete existing boolean properties.

Actual Behavior (Mandatory)

When a property on a node is already boolean it will be deleted by the normalisation.

How to Reproduce the Problem

Simple Dataset (where it's possibile)

CREATE (:Person {prop: 'Y', name:'A'}),
       (:Person {prop: 'Yes', name:'B'}),
       (:Person {prop: 'NO', name:'C'}),
       (:Person {prop: 'X', name:'D'}),
       (:Person {prop: false, name:'Valid'});

Steps (Mandatory)

  1. Check 5 people with a property prop and a bunch of values: match (n:Person) where n.prop is not null return n.name, n.prop;
  2. Apply transformation:
MATCH (n)
CALL apoc.refactor.normalizeAsBoolean(n,'prop',['Y','Yes'],['NO'])
WITH n
ORDER BY n.id
RETURN n.prop AS prop
  1. Property is gone on the person named Valid: match (n:Person) where n.prop is not null return n.name, n.prop;

While the labs page say "The other properties that don’t match these possibilities will be set as null." I find this behaviour at least surprising for things that are already boolean. And not in a good way surprising.

@michael-simons michael-simons changed the title Normalize as boolean deletes properties. Normalize as boolean deletes boolean properties, too. May 24, 2023
@Lojjs
Copy link
Contributor

Lojjs commented May 25, 2023

@michael-simons I agree this sounds odd, we will investigate

@Lojjs Lojjs added the bug label May 26, 2023
@Lojjs
Copy link
Contributor

Lojjs commented Jun 13, 2023

@michael-simons After looking into the code, it seems like this is actually intentional even though it is confusing. What you need to do is

MATCH (n)
CALL apoc.refactor.normalizeAsBoolean(n,'prop',['Y','Yes',true],['NO',false])
WITH n
ORDER BY n.id
RETURN n.prop AS prop

Since there can be people relying on the old behaviour, we decided not to change it, but we will update the documentation to explicitly mention that you need to include true and false in the parameters if you want to keep them

@Lojjs Lojjs added docs and removed bug labels Jun 13, 2023
@vga91 vga91 added the core-functionality Adding new procedure, function or signature to APOC core label Aug 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core-functionality Adding new procedure, function or signature to APOC core docs
Projects
None yet
Development

No branches or pull requests

3 participants