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 index type of FlattenNode as LargeInteger #661

Merged
merged 3 commits into from Jun 1, 2023

Conversation

DRovara
Copy link
Collaborator

@DRovara DRovara commented May 22, 2023

Currently, the getIndexVariableType() method returns Optional.empty().
However, this can cause problems: When used as part of an IRI template, the variable has to be cast to STRING. Since we don't know what the base-type is, it will use a DefaultSimpleDBCastFunctionSymbol. For this function symbol,
isAlwaysInjectiveInTheAbsenceOfNonInjectiveFunctionalTerms() returns false if the input type is unknown.

This way, the analyzeInjectivity(...) method in FunctionSymbolImpl will not return a decomposition; therefore, the ConstructionNode will not know that the Unique Constraint that may have been holding before the IRI construction will still hold, which prevents the query from being optimised.

Adding an expected type for the index variable prevents all of this, and the unique constraint will be carried over.

I chose DBLargeInteger as the type, as this is the most reasonable one. It works with all currently supported databases, and there is no reason to assume an index of the FLATTEN function for any different database may be of a non-integer type.

@DRovara DRovara requested a review from bcogrel May 22, 2023 15:14
@DRovara DRovara self-assigned this May 22, 2023
@bcogrel bcogrel marked this pull request as ready for review June 1, 2023 09:36
@bcogrel bcogrel merged commit a3f2bb5 into version5 Jun 1, 2023
16 checks passed
@bcogrel bcogrel deleted the bugfix/flatten-lens-index-var-type branch June 1, 2023 09:36
@bcogrel
Copy link
Member

bcogrel commented Jun 1, 2023

Good catch! Thanks Damian!

@bcogrel bcogrel added this to the v5.1.0 milestone Jun 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants