jnosql icon indicating copy to clipboard operation
jnosql copied to clipboard

Improve Query Abstraction (issue/improvement)

Open amoscatelli opened this issue 3 years ago • 30 comments

Hi there, I am trying to integrate JNoSql Document abstraction but I have some problem with the spec itself I believe.

  • I want to do a simple query : A and B or C and D Where A, B, C and D are simple field predicates (es. field = value). I am using the MongoDB driver and I noticed that if I write where(fieldA).eq(valueA).and(fieldB).eq(valueB).or(fieldC).eq(valueC).and(fieldD).eq(valueD) this is actually translated into : ((A and B) or C) and D I confirmed this by printing MongoDB queries into console. I believe the left associative strategy it's not the best to represent every possibile query scenario. Is this intended ? How can I do a query like (A and B) or (C and D) ? Why don't we create a proper Predicate abstraction similar to the one from JPA specification ? I expected a more hierarchical approach, to be able to do something like where(predicates) and predicates being a Collection of Predicate where a Predicate can be a Conjunctive (AND) or a Disjunctive (OR) or etc etc.

  • Also we are completely missing the possibily to UPDATE by query/condition. I mean updating every document setting a field to a certain value where a condition is specified. Is this intended too ? All of this is completed supported by MongoDB. Are there other Document databases who don't support these two features ?

I can/want to contribute on both of these requirement if we do agree on them.

I tried to subscribe to the dev mailing list but I was not accepted so far.

What do you thing about this ?

@otaviojava

amoscatelli avatar Apr 07 '22 13:04 amoscatelli

Hey, @amoscatelli it sounds like a huge challenge. Sure, please, go ahead. Please, let me know if you need any help.

otaviojava avatar Apr 07 '22 13:04 otaviojava

Hi @otaviojava . Apart from the required changes I am willing to contribute with, do you already know any reason for this to be incompatible with other Document databases (for example OrangoDB?). I never worked with other document oriented databases apart from MongoDB

amoscatelli avatar Apr 07 '22 14:04 amoscatelli

In documents databases, it should be fine. We have tests on our drivers, so it will be ok for us to test.

otaviojava avatar Apr 07 '22 14:04 otaviojava

I started working on the spec/abstraction

https://github.com/amoscatelli/nosql

Most of the work will be in the jakarta.nosql.column.criteria package, at first

amoscatelli avatar Apr 11 '22 13:04 amoscatelli

@otaviojava ok, I believe I finished the abstraction for the CriteriaQuery API. It is largely inspired by JPA, but it is indeed only a subset of those functionalities.

Tomorrow I'll finish adding comments to every interface, then I'll make a pull request and I'll start working on the eclipse implementation. At last, I will work on the MongoDB driver.

Please have a look and tell me if you like the direction I took. Also notice I will integrate a sort of metamodel generator in the eclipse implementation, scanning for @Entity and creating the metamodel on its own.

This is only the first part of what I am thinking of. CriteriaUpdate and CriteriaDelete will follow.

amoscatelli avatar Apr 11 '22 17:04 amoscatelli

I want to stress out tha fact that the Metamodel is a requirement to use the api.

amoscatelli avatar Apr 11 '22 17:04 amoscatelli

@amoscatelli you did not send the PR to review.

otaviojava avatar Apr 12 '22 09:04 otaviojava

@otaviojava here you are, I was finishing the comments before making a pull request

amoscatelli avatar Apr 12 '22 10:04 amoscatelli

@amoscatelli That is great progress. Are you going to create the implementation in the JNoSQL?

otaviojava avatar Apr 12 '22 15:04 otaviojava

Yes, I am going to. I wanted to know if you agreed before moving further. What I would like NOT to do, is to update every single document driver. I am going to take care of the MongoDB one, I hope the community will take care of the others (if any change is required indeed, I am not so sure atm).

amoscatelli avatar Apr 12 '22 15:04 amoscatelli

@amoscatelli if it will implement MongoDB I believe that it would be better to start as an extension instead of the main API:

https://github.com/eclipse/jnosql-mapping-extension

otaviojava avatar Apr 12 '22 15:04 otaviojava

Please, let me know what do you think.

otaviojava avatar Apr 12 '22 15:04 otaviojava

I am not sure I understand what you are saying. Didn't we agree that this abstraction is compatible with all Document dbs ? I added a method on the DocumentTemplate interface, is that wrong ? Also, I believe MongoDB drivers will need a change since it uses left associative strategy by default.

Anyway, if you want me to start as an extension is ok for me, please guide me on how to proceed and how to test the extension as I develop it.

What do I need to do next ?

amoscatelli avatar Apr 12 '22 15:04 amoscatelli

@amoscatelli I'm thinking :thinking: In fork; https://github.com/eclipse/jnosql-mapping-extension

  1. create a module call: criteria
  2. move the whole API code there
  3. Create a MongoDB module
  4. Implement there

What do you think?

otaviojava avatar Apr 12 '22 16:04 otaviojava

@otaviojava Sorry, I still don't get it. Do you want me to fork https://github.com/eclipse/jnosql-mapping-extension ? What do you mean by module call ? Can you show me an example of what I am supposed to do ?

amoscatelli avatar Apr 13 '22 10:04 amoscatelli

in https://github.com/eclipse/jnosql-mapping-extension I can see only db specific packages. Where should I move the jakarta.nosql.criteria package ?

amoscatelli avatar Apr 13 '22 10:04 amoscatelli

As the first step, yes. we can use it there to get mature and explore, even, to receive more feedback from the community, then, move it to the core API.

otaviojava avatar Apr 13 '22 10:04 otaviojava

@amoscatelli sorry, I did check the code again. It makes sense to keep on the core as you did. As soon, you create it to MongoDB we can merge both. Please, let me know what you think. Again, sorry for that.

otaviojava avatar Apr 14 '22 08:04 otaviojava

Today I'll start working on it

amoscatelli avatar Apr 14 '22 08:04 amoscatelli

@amoscatelli congrats on the progress, but in the end, it should return a DocumentQuery

Otherwise, it won't be compatible with what we have. You can use the DocumentCondition

E.g.: the (A and B) or (C and D)

DocumentCondition a = DocumentCondition.eq(Document.of("name", "Poliana"));
DocumentCondition b = DocumentCondition.eq(Document.of("name", "Otavio"));
DocumentCondition c = DocumentCondition.eq(Document.of("city", "São Paulo"));
DocumentCondition d = DocumentCondition.eq(Document.of("city", "Salvador"));
DocumentCondition andAB = DocumentCondition.and(a, b);
DocumentCondition andDC = DocumentCondition.and(c, d);
DocumentCondition finalCondition = DocumentCondition.or(andAB, andDC);

So, in this criteria API you can create this implementation:

 DocumentQuery query = new DocumentQuery() {
            @Override
            public long getLimit() {
                return 0;
            }

            @Override
            public long getSkip() {
                return 0;
            }

            @Override
            public String getDocumentCollection() {
                return null;
            }

            @Override
            public Optional<DocumentCondition> getCondition() {
                return Optional.empty();
            }

            @Override
            public List<Sort> getSorts() {
                return null;
            }

            @Override
            public List<String> getDocuments() {
                return null;
            }
        }

otaviojava avatar Apr 27 '22 16:04 otaviojava

Are you sure ? The new API will enable us to do things like this (I am finishing implementing this in the MongoDB driver) :

        CriteriaQuery<Person> criteriaQuery = new DefaultCriteriaQuery(Person.class);
        StringExpression<Person, Person> get = criteriaQuery.from().get(Person.NAME);
        CriteriaFunction<Person, Person, Person, Number> count = criteriaQuery.from().count();
        entityManager.executeQuery(
                criteriaQuery.select(
                        count
                ).where(
                        get.equal("Poliana")
                )
        );

Or for example I can even have a SUM operator

        CriteriaQuery<Person> criteriaQuery = new DefaultCriteriaQuery(Person.class);
        NumberExpression<Person, Person, Integer> get = criteriaQuery.from().get(Person.AGE);
        entityManager.executeQuery(
                criteriaQuery.select(
                        criteriaQuery.from().count(),
                        get.sum()
                ).where(
                        criteriaQuery.from().get(Person.NAME).equal("Poliana")
                )
        );

This is actually working, I am already finishing implementing the MongoDB driver.

Are you sure you want me to go back to the old API ? How can I expand the old API to do operations like these (count documents with predicates or do other aggregate operations) ? Please double check the API (I pushed a small update today)

amoscatelli avatar Apr 27 '22 23:04 amoscatelli

@amoscatelli

The IDE is not to change the API, the API is perfect, only the implementation. It would amazing if the CriteriaQuery<Person> returns the DocumentQuerysomehow. The main goal is to have support for all the document types, not only for MongoDB.

Remember, the main point of the specification is to support more than one database. If the goal is only to support MongoDB, initially, it is fine if it goes to extension and stays there for while.

otaviojava avatar Apr 28 '22 15:04 otaviojava

@otaviojava I think you didn't understand

I am going to make an incomplete pull request for the communication layer implementation

We'll talk about this there

amoscatelli avatar Apr 29 '22 14:04 amoscatelli

@amoscatelli what do you think if we do a call? It will make our life easier? What do you think?

otaviojava avatar Apr 29 '22 14:04 otaviojava

@otaviojava Sure it will make our life easier for this and the next functionalities (update criteria and delete criteria and I also have in mind something regarding polymoprhism).

Where would you prefer to make the call ? (telegram ? skype ? teams ?) I am available from next monday

In the mean time here is the pull request https://github.com/eclipse/jnosql-communication-driver/pull/178

amoscatelli avatar Apr 29 '22 14:04 amoscatelli

@amoscatelli please, send me an e-mail: [email protected]

btw: you did a great job.

otaviojava avatar Apr 29 '22 18:04 otaviojava

@otaviojava thank you

I sent you an email, let me know if you receive it

amoscatelli avatar Apr 30 '22 10:04 amoscatelli

Yeap, I saw yet. See you soon.

otaviojava avatar May 02 '22 08:05 otaviojava

Hey @amoscatelli Sorry for the delay

I've created the MongoDB extension and the interface to extend the particular behavior on MongoDB:

https://github.com/eclipse/jnosql-mapping-extension/blob/master/mongodb-extension/src/main/java/org/eclipse/jnosql/mapping/mongodb/MongoDBTemplate.java

The next step would be to move your code to this project and open a PR. So, we can work together. Please, let me know what you think.

otaviojava avatar May 22 '22 04:05 otaviojava

I moved the code here :

https://github.com/eclipse/jnosql-mapping-extension/pull/60#issuecomment-1134897464

Let's continue there and move it !

amoscatelli avatar May 23 '22 16:05 amoscatelli