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

(PDB-5519) Upgrade to honeysql v2 #3703

Closed
wants to merge 6 commits into from

Conversation

austb
Copy link
Contributor

@austb austb commented Aug 8, 2022

Blocked on puppetlabs/clj-parent#351 which is blocked until after the upcoming release.

@austb austb requested a review from a team as a code owner August 8, 2022 16:50
@austb austb marked this pull request as draft August 8, 2022 16:50
@austb austb changed the title Upgrade to honeysql v2 (PDB-5519) Upgrade to honeysql v2 Aug 8, 2022
@austb
Copy link
Contributor Author

austb commented Aug 8, 2022

Jenkins tests are failing because the method of pinning honeysql in this PR doesn't work when building an ezbake package. Once we get the update into clj-parent and no longer need the pin, those tests will pass.

@austb austb force-pushed the upgrade-honeysql-main branch 4 times, most recently from 39e24d1 to b0b19e1 Compare August 22, 2022 18:29
@austb austb marked this pull request as ready for review August 22, 2022 19:07
@austb austb removed the don't merge label Aug 22, 2022
@jonathannewman
Copy link
Contributor

@austb are we ready to get this in?

@austb
Copy link
Contributor Author

austb commented Sep 16, 2022

I had to make some substantive changes to our handling of group by/order handling, so I've been hoping to get @rbrw eyes on it and our PTO has been conflicting for the past weeks.

@jonathannewman
Copy link
Contributor

I think continuing to target 8.x (main) for this change makes sense. Seems risky for 7.x / LTS.

@austb austb force-pushed the upgrade-honeysql-main branch 6 times, most recently from 8ea9147 to aeb4f6a Compare October 19, 2022 21:41
@austb austb force-pushed the upgrade-honeysql-main branch 2 times, most recently from 9cc6ab7 to c78b9cd Compare October 31, 2022 15:44
honeysql v1 and v2 can be present in a project simultaneously
Updates many honeysql v1 function calls to their new v2 format. For
example, `(hcore/raw str...)` becomes `[:raw str...]`, `hcore/inline`
becomes `[:inline ...]`, etc. In a few places we need to get the string
out of a SqlRaw type (so we use `:s`), now this is the second field in
an array.

Enable native support for postgresql operators by including
`honey.sql.pg-ops`. Where trivial (and supported by our current
parameter handling), some raws are converted to native honeysql v2
syntax now that their operators are supported natively. Most are left
raw, but honeysql should support all our use-cases natively.  Checking
for a raw value is different due to the format change as well.

Update the type coercion matrix. Turning a column into a raw statement
(even the new `[:raw _]` form) is not the "noop" it was being used for
in the type coercion matrix. It causes issues when the coerced type is
used in a selection field because `{:select [^SqlRaw "string"]}` would
become `{:select [[:raw "string"]]}` which compiles as "SELECT raw as
\"string\"" instead of the desired outcome. Instead we use keyword to
"convert" the column into itself. It should be noted that this is _only_
safe due to the fact that we only use type coercion on the column name.

Update `sql-cast` which instead of creating raw SQL calls for the
conversions, uses the built in method of calling functions in honeysql
v2.

`sql-regexp-array-match-v2` is added as the new honeysql v2 version to
avoid making changes to the legacy query engine.
Before, we were compiling paging options manually and tacking them onto
the end of a query. Limit and offset will always be an integer, but
order by could include honeysql to represent the field that we are
grouping by. Honeysql v2 doesn't compile partial expressions, and
instead errors. Therefore we instead pass the paging options through
with our generated query plan and allow honeysql to create the paging
options in the query directly.
hsql v1 allowed for `:modifiers` to specify the distinct query, v2
requires that you use `:select-distinct` instead.
All usage of honeysql v1 has been removed
@austb austb requested a review from a team as a code owner October 31, 2022 18:49
@austb austb closed this Nov 1, 2022
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

Successfully merging this pull request may close these issues.

None yet

2 participants