Closed Brayden closed 4 months ago
Discussion before we merge this PR in. What is the exact output we want from this .toString()
function? Is it:
INSERT INTO person (id, first_name, last_name) VALUES (:id, :first_name, :last_name)
or
INSERT INTO person (id, first_name, last_name) VALUES (1, "John", "Doe")
So, this PR kind of exploded in scope a bit. Originally we wanted to just add the ability for the query builder to support a .toString()
function that instead of executing the query could spit out what it was generating and about to run. What actually ended up happening was to make this happen a bit of refactoring was needed.
The refactoring that took shape made sense to also introduce some Jest tests to make sure what was being modified was not altering the ability or output of the query builder itself. Why would it, you might ask? Well different database API's and drivers take in queries differently – either by named parameters or positional parameters in the query. Previously the query builder only produced a query with named parameters. The Cloudflare implementation had special logic to try to convert it from named to positional, but this was glaringly a change that needed to be global.
So here we are today. Now connections can identify themselves as requiring either named
or positional
query parameters. When the query builder is utilized to construct a SQL statement it will generate the appropriate type of query based on that connection property.
A couple of new types exist:
export type Query = {
query: string
parameters?: QueryParams
}
export type QueryParamsNamed = Record<string, any>
export type QueryParamsPositional = any[]
export type QueryParams = QueryParamsNamed | QueryParamsPositional
A query typed object now contains both a query string and a parameters object that maps either to a named or positional query type. This helps tell it how to fill in the blanks properly when constructing a .toString()
operation, for example.
Tests are added to help build confidence in the changes. We should continue to add more tests as we go and get more granular, specific, and in depth with them.
@caleb-mabry Updated! Also added a new test to make sure that the npm exec sync-database-models
command generates an output we expect. Great catch 🚀
Purpose
Per the attached issue, the goal of this pull request is to add the ability for the query builder chain to respond with a SQL statement by using the
.toString()
function call at the end.Tasks
.toString()
that returns a SQL statement.toString()
functionVerify
See output of
data
object as:Before
N/A
After
N/A