-
Notifications
You must be signed in to change notification settings - Fork 4.2k
added simple sanitizer methods for nodes and realtionships #3021
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 simple sanitizer methods for nodes and realtionships #3021
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey! Thanks for your contribution.
I have noticed that the _remove_spaces_from_entities
function now includes additional sanitization logic like title-casing node labels and upper-casing relationships (via _sanitize_node_label
and _sanitize_relationship_type
).
This introduces a few concerns:
Breaking change risk: We currently store and match entity names using lowercase with underscores (e.g., customer_account). Changing this format might cause entity mismatches or duplicate nodes in Neo4j.
Inconsistent mapping: The entity_type_map and various Cypher queries assume the previous normalized format, and updating one side without the other could lead to silent failures.
Would you mind reverting to the original space-to-underscore normalization for now and possibly exploring a safer transition plan separately?
Hey @akshat1423, The space to _ will stay. You can expect the the new pr soon. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Thank you for the contributions.
Description
Cypher code was breaking if "-" was in the name of relation. Same with non-ASCII characters.
Added simple sanitiser for relationship and node names.
New node and relation names follow neo4j convention (UPPERCASE and PascalCase, respectively)
Fixes #3020
Type of change
How Has This Been Tested?
Manually tested. Try imputing the prompts form the issue above and log the results. :)
Checklist:
Maintainer Checklist