search

andywer / squid

🦑 Provides SQL tagged template strings and schema definition functions.
MIT License
148 stars 7 forks source link

Refactor building SQL values #30

Closed brandonchinn178 closed 4 years ago

brandonchinn178 commented 4 years ago

This PR contains no user-facing changes, it's only internal refactoring. That said, it's a huge change, and rewrites a lot of the internal mechanism for generating SQL queries. In my opinion, this PR takes the library from good to great. I'm happy to discuss any of these changes and iterate towards something beneficial for everyone.

The primary motivation for this was #27, but even aside from that PR, these changes make it much easier for me, someone completely new to the codebase, to understand what's going on. The primary intuition behind the sql/spreadAnd/... functions is now to convert everything into a new SqlBuilder type (either as a SqlBuilder that returns just the raw string or as a SqlBuilder that substitutes the value in with placeholders) and then use utilities like sqlBuild or joinSql to stitch them together.

Again, the point of this PR is to make the codebase easier to skim and quickly understand. If there's anything in here that makes the code more difficult to understand, happy to discuss.

andywer commented 4 years ago

Looks promising! Thanks so much for your efforts already, @brandonchinn178 😊

I need a little bit more time to go through it in detail, but that looks sound on first glance. I might have stumbled into micro-optimizations too quickly – you are totally right, at this point it's definitely more important to make the code easy to understand.