lilactown / pyramid

A library for storing and querying graph data in Clojure
Eclipse Public License 2.0
228 stars 10 forks source link

Reverse order for lists? #24

Closed galaux closed 1 year ago

galaux commented 2 years ago

Hey @lilactown !

Thanks for this great library.

I was wondering whether lists being reversed was a known/expected behavior or if this could be a bug or misuse on my side:

(let [entity {:id 9
              :my-list [{:thing {:id 1}}  ;; ← vector
                        {:thing {:id 2}}
                        {:thing {:id 3}}]}
      db (pyramid.core/add {} entity)]
  (pyramid.core/pull db [{[:id 9] [:id {:my-list [{:thing [:id]}]}]}]))
;; Order is preserved: {[:id 9] {:id 9, :my-list [{:thing {:id 1}} {:thing {:id 2}} {:thing {:id 3}}]}}

(let [entity {:id 9
              :my-list '({:thing {:id 1}}  ;; ← list
                         {:thing {:id 2}}
                         {:thing {:id 3}})}
      db (pyramid.core/add {} entity)]
  (pyramid.core/pull db [{[:id 9] [:id {:my-list [{:thing [:id]}]}]}]))
;; Reverse order: {[:id 9] {:id 9, :my-list ({:thing {:id 3}} {:thing {:id 2}} {:thing {:id 1}})}}

I noted the README says:

Collections like vectors, sets and lists should not mix entities and non-entities. Collections are recursively walked to find entities.

Could this be the explanation? I'm not sure I fully understand what exactly should be avoided.

Thanks for your time.

lilactown commented 2 years ago

Thanks for the report. Yup, this is a bug!

lilactown commented 2 years ago

you can try 3.3.0-SNAPSHOT, it ought to fix it for you

galaux commented 2 years ago

Unfortunately no, results are still reversed (in my app and in the snippet provided above).

I double checked I'm using the correct version:

❯ tree ~/.m2/repository/town/
/home/galaux/.m2/repository/town/
└── lilac
    ├── cascade
    │   └── 2.0.0
    │       ├── cascade-2.0.0.jar
    │       ├── cascade-2.0.0.jar.sha1
    │       ├── cascade-2.0.0.pom
    │       ├── cascade-2.0.0.pom.sha1
    │       └── _remote.repositories
    └── pyramid
        └── 3.3.0-SNAPSHOT
            ├── maven-metadata-clojars.xml
            ├── maven-metadata-clojars.xml.sha1
            ├── pyramid-3.3.0-20220611.195651-2.jar
            ├── pyramid-3.3.0-20220611.195651-2.jar.sha1
            ├── pyramid-3.3.0-20220611.195651-2.pom
            ├── pyramid-3.3.0-20220611.195651-2.pom.sha1
            ├── pyramid-3.3.0-SNAPSHOT.jar
            ├── pyramid-3.3.0-SNAPSHOT.pom
            ├── _remote.repositories
            └── resolver-status.properties
lilactown commented 2 years ago

looks like my test got lost in a merge, and a bunch of other changes caused the fix to break. I'll keep working on it!

lilactown commented 2 years ago

@galaux try again?

galaux commented 2 years ago

@lilactown SUCCESS 🥳

Thanks for your swift action !