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

Transforms do not transform any values in insert! that are not present in the first row #130

Closed
camsaul opened this issue Mar 17, 2023 · 3 comments · Fixed by #138
Closed
Labels
bug Something isn't working good first issue Good for newcomers high priority tools.transforms

Comments

@camsaul
Copy link
Owner

camsaul commented Mar 17, 2023

Not really 100% clear that this should be allowed or ultimately will work anyway, but it causes Honey SQL compilation to fail because the column is untransformed which is misleading. I spent a solid 30 minutes trying to figure out why transformation wasn't working util I figured out what was going on.

Failing test for toucan2.tools.transformed-test:

(derive ::venues.edn-category ::test/venues)

(transformed/deftransforms ::venues.edn-category
  {:category {:in  pr-str
              :out (fn [s]
                     (binding [*read-eval* false]
                       (read-string s)))}})

(deftest ^:parallel transform-insert-rows-test
  (is (= [{:name "Venue 1"}
          {:name "Venue 2", :category "{:name \"Category 2\"}"}]
         (#'toucan2.tools.transformed/transform-insert-rows
          '[{:name "Venue 1"}
            {:name "Venue 2", :category {:name "Category 2"}}]
          (#'toucan2.tools.transformed/in-transforms ::venues.edn-category)))))
@qnkhuat
Copy link
Contributor

qnkhuat commented Mar 20, 2023

bumping since I encountered this on the Metabase repo today.

If you give me some pointers, I'll try to get a PR to fix this.

@camsaul
Copy link
Owner Author

camsaul commented Mar 24, 2023

@qnkhuat you'll just need to tweak this function:

(defn- transform-insert-rows [[first-row :as rows] k->transform]
{:pre [(map? first-row) (map? k->transform)]}
;; all rows should have the same keys, so we just need to look at the keys in the first row
(let [row-xforms (for [k (keys first-row)
:let [xform (get k->transform k)]
:when xform]
(fn [row]
(update row k (fn [v]
(if (some? v)
(xform v)
v)))))
row-xform (apply comp row-xforms)]
(map row-xform rows)))

and you can copy that test in the description and put it in toucan2.tools.transformed-test.

@camsaul
Copy link
Owner Author

camsaul commented Mar 27, 2023

Fixed in 1.0.524

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers high priority tools.transforms
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants