Instrument search is case-sensitive

[louise-davies]

Description:
Will commented that the instrument search was case-sensitive. If that’s the case could it all be made case-insensitive, especially as some of the instruments have unusual ‘default’ cases.

I'm not sure if this is referring to lucene search or for instrument filters but we should investigate and resolve this.

Acceptance criteria:

• Instrument "search" is no longer case-sensitive

[GoelBiju]

As an update to this issue, the filters which are used on table/card views are all case-sensitive. A query for the instrument "SANS2D" using the instrument filter on the ISIS views using lowercase ("sans2d") would not work, neither would "Sans2D". For instruments specifically, as @louise-davies mentioned, we could just make the filter text uppercase since instruments are all uppercase in ISIS.

It would be better to have a way to have a case-insensitive search with the API query since the other fields are also case-sensitive. @MRichards99 is there a way to have case insensitivity?

[MRichards99]

I believe these filters use the like keyword to the API? Should be very simple to add a case insensitive version of that, is one needed for the not like (nlike) operator too?

[GoelBiju]

As an update to this issue, the filters which are used on table/card views are all case-sensitive. A query for the instrument "SANS2D" using the instrument filter on the ISIS views using lowercase ("sans2d") would not work, neither would "Sans2D". For instruments specifically, as @louise-davies mentioned, we could just make the filter text uppercase since instruments are all uppercase in ISIS.

It would be better to have a way to have a case-insensitive search with the API query since the other fields are also case-sensitive. @MRichards99 is there a way to have case insensitivity?

Just as a follow-up to this @MRichards99 the issue seems to be with the card view components for ISIS/DLS and not with the query itself. Every other table/card view does it correctly. It was a case of the dataKey attribute being investigationInstruments[0].instrument.fullName and not investigationInstruments.instrument.fullName.

I'll provide a fix for this which requires a minor change to the CardView component in order to correctly have the tooltip working since that uses the default dataKey value does not include the array index.

[MRichards99]

@GoelBiju thanks for the update!

[GoelBiju]

I need to do a check on the ISIS/DLS views to see if there are any issues with filtering anywhere else i.e. test out if the other filters on the views work as well.

[GoelBiju]

@MRichards99 so what @louise-davies had mentioned a couple of days before was that instrument search is not working for ISIS on the deployed data.

So this query for "SANS2D" works whereas this one for "sans2d" does not. This is not an issue with development data or when running locally, however.

[MRichards99]

@GoelBiju I've just noticed this in blocked on the sprint board - do you need anything additional from the API?

[GoelBiju]

@GoelBiju I've just noticed this in blocked on the sprint board - do you need anything additional from the API?

Yeah seems like the search on instrument name search on the ISIS instrument table view is an issue with the way we are doing the search query. I'll remove it from blocked since from @louise-davies mentioned in a meeting it could be fixed by altering the query. Need to confirm if this is the issue or not still.

[MRichards99]

Ah ok, I just wanted to make sure I wasn't holding you up if you needed a feature implementing on the API! :)

[GoelBiju]

Ah ok, I just wanted to make sure I wasn't holding you up if you needed a feature implementing on the API! :)

No problem, thanks for it @MRichards99! 😃

[GoelBiju]

@GoelBiju I've just noticed this in blocked on the sprint board - do you need anything additional from the API?

Yeah seems like the search on instrument name search on the ISIS instrument table view is an issue with the way we are doing the search query. I'll remove it from blocked since from @louise-davies mentioned in a meeting it could be fixed by altering the query. Need to confirm if this is the issue or not still.

• Investigate the difference in queries for ISIS views and TopCAT.

[sam-glendenning]

@GoelBiju this is marked as ready for review but by the sounds of it it's still being worked on. Shall I wait on your go before having a look at it?

[GoelBiju]

@GoelBiju this is marked as ready for review but by the sounds of it it's still being worked on. Shall I wait on your go before having a look at it?

@sam-glendenning I had updated since it does fix the instrument filter which was not working correctly before but there are additional issues. I have put a "WIP" on the pull request now since it is still in progress. Thanks for notifying me 😃

[GoelBiju]

Looking further into this issue, the difference between the query that we make and what TopCAT presently does is the UPPER on the field where we use the like operator. E.g. on TopCAT the following part of the main query checks if "ISIS" is in the description field (which is in uppercase): UPPER(facilityCycle.description) like concat('%', 'ISIS', '%')

@MRichards99 like we mentioned before it seems this might be something has to be done on the API?

[louise-davies]

I think the other important thing to note that we need to do this due to different databases working differently. Preprod doesn't have this issue since it's using a MariaDB database, but the proper ICATs run on Oracle databases and so this is why it's being seen on those servers.

[MRichards99]

@GoelBiju so does UPPER(facilityCycle.description) convert the data stored in the database to be uppercased or does it uppercase the value passed in the query? I'm thinking it's the first one but just want to check my understanding.

I was thinking I'd just be able to add another branch to this conditional that just passes ilike but JPQL doesn't seem to support that.

My next thought was to try and replicate the TopCAT query into the API to create our own case insensitive operator:

elif self.operation == "ilike1":
    self.field = f"UPPER({self.field})"
    where_filter = self.create_condition(self.field, "like", f"%{self.value}%")

This code results in this:

{'UPPER(title)': "like '%police%'"}

A working example using a regular like for reference:

{'title': "like '%police%'"}

Python ICAT gets confused with the UPPER keyword when it tries to get information about the entity. I'll try and have a think over the next couple of days about how this could be solved in Python ICAT.

Unless there's an Oracle DB specific solution we could implement (providing there's some way to check what kind of database is running within the API)?

[GoelBiju]

@GoelBiju so does UPPER(facilityCycle.description) convert the data stored in the database to be uppercased or does it uppercase the value passed in the query? I'm thinking it's the first one but just want to check my understanding.

I was thinking I'd just be able to add another branch to this conditional that just passes ilike but JPQL doesn't seem to support that.

My next thought was to try and replicate the TopCAT query into the API to create our own case insensitive operator:

elif self.operation == "ilike1":
    self.field = f"UPPER({self.field})"
    where_filter = self.create_condition(self.field, "like", f"%{self.value}%")

This code results in this:

{'UPPER(title)': "like '%police%'"}

A working example using a regular like for reference:

{'title': "like '%police%'"}

Python ICAT gets confused with the UPPER keyword when it tries to get information about the entity. I'll try and have a think over the next couple of days about how this could be solved in Python ICAT.

Unless there's an Oracle DB specific solution we could implement (providing there's some way to check what kind of database is running within the API)?

I think you are right @MRichards99, I am thinking it would only convert the value already stored in that field in the database to uppercase and then perform the like which the string we pass.

A small detail I noticed in the TopCAT query was the query string e.g. "isis" was also in uppercase ("ISIS") and this must have been checked against the field which was also converted to uppercase. So maybe that was the solution in place, it relied on converting both to uppercase to check? So looking online we would need to do upper/lower on both sides of the query in order to replicate this.

Would we need to do the upper/lower on the actual query itself and not the condition that is returned?

[MRichards99]

I would also expect both sides would need to be uppercase (so LHS=RHS, good old algebra...) but in testing I didn't really experience that.

Notice how my query uppercases the title but the value is a lowercase "police". Despite this, it still picks up titles which have "police" as lowercase:

If I change the query to SELECT o FROM Investigation o WHERE lower(o.title) like '%POLICE%', then it still picks up that investigation.

The condition (a Python ICAT term, it's essentially a where filter in DataGateway world) is part of the query - Python ICAT combines all these different things (basically the entity of the query and any of DataGateway API's filters) and converts them into a JPQL string to be sent off to ICAT. To replicate the TopCAT solution, the field name needs to have UPPER() wrapped around, but this is something Python ICAT doesn't like. Based on my (short) testing, the API might not need to wrap UPPER() on the value (i.e. police or ISIS). If we do need to UPPER() the value, this can be implemented into the API very easily via a new where filter operator, I've just tested that Python ICAT already supports it:

where_filter = self.create_condition(self.field, "like", f"UPPER(%{self.value}%)")

[GoelBiju]

I would also expect both sides would need to be uppercase (so LHS=RHS, good old algebra...) but in testing I didn't really experience that.

Notice how my query uppercases the title but the value is a lowercase "police". Despite this, it still picks up titles which have "police" as lowercase:

If I change the query to SELECT o FROM Investigation o WHERE lower(o.title) like '%POLICE%', then it still picks up that investigation.

The condition (a Python ICAT term, it's essentially a where filter in DataGateway world) is part of the query - Python ICAT combines all these different things (basically the entity of the query and any of DataGateway API's filters) and converts them into a JPQL string to be sent off to ICAT. To replicate the TopCAT solution, the field name needs to have UPPER() wrapped around, but this is something Python ICAT doesn't like. Based on my (short) testing, the API might not need to wrap UPPER() on the value (i.e. police or ISIS). If we do need to UPPER() the value, this can be implemented into the API very easily via a new where filter operator, I've just tested that Python ICAT already supports it:

where_filter = self.create_condition(self.field, "like", f"UPPER(%{self.value}%)")

That's great @MRichards99, I was initially thinking that having upper on one side means the opposite would not work. Since it is working with lower applied to the field then that's brilliant!

[MRichards99]

@GoelBiju I've just tested on ISIS dev and UPPER() is required on both sides. I've been testing using the entity manager endpoint:

SELECT o FROM Investigation o WHERE UPPER(o.title) like UPPER('%toMAto%')

The UPPER() on the value side isn't a problem but good we know now.

[GoelBiju]

@GoelBiju I've just tested on ISIS dev and UPPER() is required on both sides. I've been testing using the entity manager endpoint:

SELECT o FROM Investigation o WHERE UPPER(o.title) like UPPER('%toMAto%')

The UPPER() on the value side isn't a problem but good we know now.

That's great @MRichards99, hopefully, we can figure out how to apply the UPPER to the field.

[MRichards99]

I think I've found a solution for this, with some small changes on Python ICAT and perhaps an additional operator for DataGateway API's where filter (we can have the changes into like so that's always case insensitive or have a separate ilike operator and change DataGateway's operator use from like to ilike, this should be discussed further). Will put an issue and some commits to Rolf for him to look at.

[MRichards99]

I've opened an issue for this (icatproject/python-icat#86) and waiting on permissions so I can commit my changes and create a PR for them.

[MRichards99]

Status update: I've created a PR for my suggested Python ICAT changes (icatproject/python-icat#87).

For DataGateway API, there are two options - I can make the existing like operator case insensitive (by applying UPPER() on both field and value) or I can make a new operator, ilike:

elif self.operation == "ilike":
    self.field = f"UPPER({self.field})"
    where_filter = self.create_condition(
        self.field, "like", f"UPPER('%{self.value}%')",
    )

I'm happy to do this, but I'm also conscious doing this would mean all like operators used in the frontend would need to be changed to ilike. We should decide whether we want a new operator or modifying the existing one is sufficient.

[sam-glendenning]

Are there many instances in the frontend which would require case sensitivity in the query? I can't think of any. Unless anyone comes up with something that'll break everything, I'd support changing the like operator to support case-insensitivity

[GoelBiju]

That's great @MRichards99 that there is a solution now. I think it would be nice to have it configurable by having a separate operator but like @sam-glendenning if we do not have a case where we need case sensitivity then it might be better to support case-insensitivity by default.

[MRichards99]

I can't think of anything, one for @louise-davies perhaps

[louise-davies]

I think for now we just change the default - we can always add back in a "case sensitive" oeprator later if needed as I can';t imagine a use for one right now

[MRichards99]

Once the changes on Python ICAT have been finalised, I'll edit the existing like operator on the API as per the linked issue above.