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

criteria-geode backend toLowerCase/toUpperCase issue #1189

Open
garrettlestingi opened this issue Jun 17, 2020 · 5 comments
Open

criteria-geode backend toLowerCase/toUpperCase issue #1189

garrettlestingi opened this issue Jun 17, 2020 · 5 comments

Comments

@garrettlestingi
Copy link
Contributor

It seems that when using the toUpperCase and toLowerCase String operators parentheses are needed like so: name.toLowerCase()

Here is an example that I drew up to output the translated OQL and show the queries that are not working. I am using this geode version 1.12 docker image to start my locator/server, creating a "person" region, and connecting with the following client example using a "PROXY" region:

public class Main {
    private static final Logger logger = Logger.getLogger(Main.class.getName());

    public static void main(String[] args) {
        GemFireCache cache = new ClientCacheFactory().create();
        GeodeBackend backend = new GeodeBackend(GeodeSetup.of(cache));
        PersonRepository repository = new PersonRepository(backend);

        repository.insert(new Person()
                .withId("id1")
                .withName("garrett")
                .withHobbies(Collections.emptyList()));
        repository.insert(new Person()
                .withId("id2")
                .withName("GARRETT")
                .withHobbies(Collections.singletonList("hobby")));

        List<Person> results = repository
                .find(PersonCriteria.person.name.toLowerCase().is("garrett"))
                .fetch();
        logger.info("toLowerCase() number of results: " + results.size());

        results = repository
                .find(PersonCriteria.person.name.toUpperCase().is("GARRETT"))
                .fetch();
        logger.info("toUpperCase() number of results: " + results.size());

        results = repository
                .find(PersonCriteria.person.name.is("garrett"))
                .fetch();
        logger.info("is number of results: " + results.size());
    }

Log output with loglevel FINE for criteria:

Jun 16, 2020 3:00:00 PM org.immutables.criteria.geode.SyncSelect call
FINE: Querying Geode with oql=[SELECT * FROM /person WHERE name.toLowerCase = $1]  with 1 variables [$1=garrett]
Jun 16, 2020 3:00:00 PM Main main
INFO: toLowerCase() number of results: 0
Jun 16, 2020 3:00:00 PM org.immutables.criteria.geode.SyncSelect call
FINE: Querying Geode with oql=[SELECT * FROM /person WHERE name.toUpperCase = $1]  with 1 variables [$1=GARRETT]
Jun 16, 2020 3:00:00 PM Main main
INFO: toUpperCase() number of results: 0
Jun 16, 2020 3:00:00 PM org.immutables.criteria.geode.SyncSelect call
FINE: Querying Geode with oql=[SELECT * FROM /person WHERE name = $1]  with 1 variables [$1=garrett]
Jun 16, 2020 3:00:00 PM Main main
INFO: is number of results: 1

The following query run from gfsh DOES return expected results (after running the above to insert the two rows). Note the parentheses below vs the above criteria log output:

gfsh>query --query="select count(*) from /person where name.toLowerCase() = 'garrett'"
Result : true
Rows   : 1

Result
------
2

Interestingly, I ran the same example Main.java above using a "LOCAL" region in my example client and I get the expected results, which may explain why this issue was missed:

Jun 16, 2020 10:33:58 PM org.immutables.criteria.geode.SyncSelect call
FINE: Querying Geode with oql=[SELECT * FROM /person WHERE name.toLowerCase = $1]  with 1 variables [$1=garrett]
Jun 16, 2020 10:33:58 PM Main main
INFO: toLowerCase() number of results: 2
Jun 16, 2020 10:33:58 PM org.immutables.criteria.geode.SyncSelect call
FINE: Querying Geode with oql=[SELECT * FROM /person WHERE name.toUpperCase = $1]  with 1 variables [$1=GARRETT]
Jun 16, 2020 10:33:58 PM Main main
INFO: toUpperCase() number of results: 2
Jun 16, 2020 10:33:58 PM org.immutables.criteria.geode.SyncSelect call
FINE: Querying Geode with oql=[SELECT * FROM /person WHERE name = $1]  with 1 variables [$1=garrett]
Jun 16, 2020 10:33:58 PM Main main
INFO: is number of results: 1

My proposal is to change the below group of unary operators in GeodeVisitor.java to include parentheses which works in all cases. I think it makes sense to keep this group together for consistency even though the IterableOpterators.IS_EMPTY is included and works without parentheses:

    }  else if (op == IterableOperators.IS_EMPTY || op == StringOperators.TO_LOWER_CASE || op == StringOperators.TO_UPPER_CASE) {
      return oql(arg0.accept(this).oql() + "." + toMethodName(op));
    }
@asereda-gs
Copy link
Member

PR merged.
So the conclusion is that queries behave differently for LOCAL vs PROXY caches if parentheses (()) are used or not ?

@asereda-gs
Copy link
Member

Can you pls also check how things behave for enums ? name vs name() vs toString() ?
See 0f5315b (enums without bind variables)

@garrettlestingi
Copy link
Contributor Author

garrettlestingi commented Jun 17, 2020

Yes, I'll summarize the tests and results below:

After inserting the following rows:

        repository.insert(new Person()
                .withId("id1")
                .withName("garrett")
                .withHobbies(Collections.emptyList()));
        repository.insert(new Person()
                .withId("id2")
                .withName("GARRETT")
                .withHobbies(Collections.singletonList("hobby")));

        List<Person> results = repository
                .find(PersonCriteria.person.name.toLowerCase().is("garrett"))
                .fetch();

I tested all combinations of using a LOCAL vs PROXY region and with/without parentheses in the OQL. Here are the results

LOCAL PROXY
without parentheses 2 rows returned 0 rows returned
with parentheses 2 rows returned 2 rows returned

@garrettlestingi
Copy link
Contributor Author

0f5315b

Absolutely, I'll do a similar test using enums with/without bind variables, using LOCAL vs PROXY regions

@asereda-gs
Copy link
Member

Also see method invocation from gemfire documentation:

.. If you know that the attribute name maps to a public method that takes no parameters, you can simply include the method name in the query string as an attribute. For example, emps.isEmpty is equivalent to emps.isEmpty() ...

Go figure ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants