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.

Sign up for GitHub

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

Jump to bottom

[hZww9vSJ] Make the type output of apoc.schema.* procedures more specific #307

Merged
merged 3 commits into from
Feb 13, 2023
Merged

[hZww9vSJ] Make the type output of apoc.schema.* procedures more specific #307

merged 3 commits into from
Feb 13, 2023

Conversation

vga91
Copy link
Collaborator

@vga91 vga91 commented Jan 27, 2023 •
edited
Loading

Changed ConstraintDescriptor to ConstraintDefinition in nodeInfoFromConstraintDefinition.
This to be consistent with relationshipInfoFromConstraintDefinition and to retrieve the constraint type with constraintDefinition.getConstraintType().name(), easier to implement and not internal.
The only con is that to get the "userDescriptor" output I have to use this method ktx.schemaRead().constraintGetForName(constraintDefinition.getName()).userDescription(tokens).

Alternatively, I could remain ConstraintDescriptor and derive the type of constraint via a method like this, but it's not future-proof:

public static ConstraintType getConstraintType(ConstraintDescriptor descriptor) {
    if (descriptor.isNodeUniquenessConstraint()) return ConstraintType.UNIQUENESS;
    if (descriptor.isNodeKeyConstraint()) return ConstraintType.NODE_KEY;
    if (descriptor.isNodePropertyExistenceConstraint()) return ConstraintType.NODE_PROPERTY_EXISTENCE;
...
    return ConstraintType.UNIQUENESS;
}

With this implementation, a uniqueness constraint returns 2 results (1 index "RANGE" and a constraint "UNIQUENESS"), consistently to relationships (previously returned a generic "INDEX" result).

I don't think it's a breaking-change but rather a bug as the documentation says returns ALL indexes and constraints information for all node labels in the database..
If in doubt, I can create a card and maybe discuss it together.

  • changed indexDescriptor.isUnique() ? "UNIQUENESS" : "INDEX"; to indexDescriptor.getIndexType().name();
  • removed hardcoded "INDEX" from type output
  • Simplified Schemas.relationshipInfoFromIndexDescription

@vga91 vga91 marked this pull request as draft January 27, 2023 17:04
@vga91 vga91 force-pushed the apoc.schemas-output-more-specific branch from 98e794d to 3307ed5 Compare January 27, 2023 21:52
@vga91 vga91 force-pushed the apoc.schemas-output-more-specific branch from 4e1c078 to 4f0fc6a Compare January 30, 2023 08:15
@vga91 vga91 marked this pull request as ready for review January 30, 2023 08:27
gem-neo4j
gem-neo4j approved these changes Feb 1, 2023
View reviewed changes
Copy link
Contributor

@gem-neo4j gem-neo4j left a comment

Choose a reason for hiding this comment

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

Looks really good :) Can you please update the docs as well to show the correct type (i.e not uniqueness), just one small comment otherwise! Nice :D
I also agree this isn't breaking as it is a bug fix!

core/src/test/java/apoc/schema/SchemasTest.java Outdated
result -> {
while (result.hasNext()) {
Map<String, Object> r = result.next();
System.out.println("r = " + r);
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably best to remove print statements now :)

@vga91
Copy link
Collaborator Author

vga91 commented Feb 8, 2023

@gem-neo4j I added the docs pr: https://github.com/neo4j/docs-apoc/pull/107
I'll merge this one as soon as it turns green :)

@gem-neo4j
Copy link
Contributor

Great, I'll review the docs when I have time, thanks! :D

@vga91 vga91 merged commit 971d823 into dev Feb 13, 2023
@vga91 vga91 deleted the apoc.schemas-output-more-specific branch February 13, 2023 09:33
vga91 added a commit to neo4j-contrib/neo4j-apoc-procedures that referenced this pull request Jun 20, 2023
@vga91
[hZww9vSJ] Make the type output of apoc.schema.* procedures more spec…
36d0c04 
…ific (neo4j/apoc#307)

* [hZww9vSJ] Make the type output of apoc.schema.* procedures more specific

* [hZww9vSJ] Fix some SchemasEnterpriseFeaturesTest

* [hZww9vSJ] Remove println
vga91 added a commit to neo4j-contrib/neo4j-apoc-procedures that referenced this pull request Jun 20, 2023
@vga91
[hZww9vSJ] Make the type output of apoc.schema.* procedures more spec…
c56326c 
…ific (neo4j/apoc#307) (neo4j/docs-apoc#107) (#3635)

* [hZww9vSJ] Make the type output of apoc.schema.* procedures more specific (neo4j/apoc#307)

* [hZww9vSJ] Make the type output of apoc.schema.* procedures more specific

* [hZww9vSJ] Fix some SchemasEnterpriseFeaturesTest

* [hZww9vSJ] Remove println

* [hZww9vSJ] Make the type output of apoc.schema.* procedures more specific (neo4j/docs-apoc#107)

* [hZww9vSJ] Make the type output of apoc.schema.* procedures more specific

* [hZww9vSJ] var. output description changes, moved config, added tables

* [hZww9vSJ] improved populationProgress description

* [hZww9vSJ] small changes

* [hZww9vSJ] cypher style and some small changes - changed note for exclude/include config

* [hZww9vSJ] small change

* [hZww9vSJ] removed unused imports
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gem-neo4j gem-neo4j gem-neo4j approved these changes

Assignees

@gem-neo4j gem-neo4j

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@vga91 @gem-neo4j