search

tursodatabase / libsql-client-go

Go client API for libSQL
MIT License
159 stars 23 forks source link

feat(params): add full support for all sqlite parameter templates #120

Closed guergabo closed 2 weeks ago

guergabo commented 1 month ago

Problem

The database driver currently supports unnamed parameters (?) and named parameters (:AAAA, @AAAA, $AAAA). However, there is a missing feature for positional parameters with indexes (?NNN) that has been asked for by many users (see issues).

This PR solves the following issues:

Fixes https://github.com/tursodatabase/libsql-client-go/issues/100 Fixes https://github.com/tursodatabase/libsql-client-go/issues/106

Context

Initially, based on comments indicating that positional parameters with indexes were not supported, I assumed this meant they were not supported at all on the server side. The Hrana protocol seemed to lack distinction for positional arguments with indexes, unlike named arguments. Furthermore, the code check positionalParametersOffset+positionalParamsCount > len(queryParams.positional) suggested the same.

type Stmt = {
    "sql"?: string | null,
    "sql_id"?: int32 | null,
    "args"?: Array<Value>,
    "named_args"?: Array<NamedArg>,
    "want_rows"?: boolean,
}

type NamedArg = {
    "name": string,
    "value": Value,
}

As a result, my first attempt involved having the client unpack queries with positional parameters with indexes into regular positional parameters.

query: "select * from x where y = ?2 and z = ?1" args: "a, b"

becomes...

query: select * from x where y = ? and z = ? args: "b, a"

However, upon investigating the libsql codebase, I discovered that the server already supported positional parameters with indexes, as evidenced by the following code:

More over the WebSocket driver implementation does not have the same checks that the HTTP driver implementation does and therefore while manually testing with sqld could see that positional parameter with indexes already worked for the WebSocket variant.

Solution

This meant that what had to be done was to just update the http implementation to allow positional parameters with indexes and adjust the code accordingly to maintain existing behavior.

Tests were added for both WebSocket and HTTP variants.