observablehq / stdlib

The Observable standard library.
https://observablehq.com/@observablehq/standard-library
ISC License
957 stars 83 forks source link

String templates don't work in DuckDB client #321

Closed domoritz closed 1 year ago

domoritz commented 1 year ago

See https://observablehq.com/@cmudig/duckdb-sql-cells.

Screenshot 2022-11-11 at 12 42 33
mbostock commented 1 year ago

I will investigate.

mbostock commented 1 year ago

I was following the pattern from your DuckDBClient’s queryRow implementation:

// query a single row
async queryRow(query, params) {
  const key = `Query ${this._counter++}: ${query}`;

  console.time(key);
  const conn = await this.connection();
  // use send as we can stop iterating after we get the first batch
  const result = await conn.send(query, params);
  const batch = (await result.next()).value;
  console.timeEnd(key);

  return batch?.get(0);
}

But it looks like connection.send only takes a single argument, so the params are dropped. I need to figure out how to pass the parameters.

domoritz commented 1 year ago

Thank you for the quick fix!

domoritz commented 1 year ago

Is there anything I need to do to refresh the standard library or do I just wait (https://observablehq.com/@cmudig/duckdb-sql-cells?collection=@cmudig/duckdb)?

The broader question is how you deal with version updates in general (e.g. going to Arrow 9's API in the standard library)?

mbostock commented 1 year ago

It’s working correctly for me now, but you need to modify your query to remove the quotes:

SELECT avg(precipitation)
  FROM weather
  WHERE location == ${city}

You can’t interpolate dynamic parameters into string literals; you can only reference them as variables.

You can reproduce the error with the AsyncDuckDB directly like so:

{
  const connection = await client._db.connect();
  try {
    const statement = await connection.prepare(`SELECT avg(precipitation) FROM weather WHERE location == "?"`);
    return await statement.query(city);
  } finally {
    await connection.close();
  }
}

I believe the error means that it’s looking for a column literally named ?, which of course doesn’t exist. (The double quotes are used to escape the name and disambiguate from parameter substitution.)

If you were expecting this to work, let me know what the right sequence of AsyncDuckDB calls are and I’m happy to change the DuckDBClient implementation accordingly.

mbostock commented 1 year ago

The broader question is how you deal with version updates in general (e.g. going to Arrow 9's API in the standard library)?

As a workaround for now you can load an Apache Arrow file by passing a version argument to the arrow method:

gaia = FileAttachment("gaia-sample.arrow").arrow({version: 9})

We currently support versions 9 (used by DuckDBClient) and versions 4 (used by the versions of Arrow and aq currently exposed in the standard library).

In the near future we will be adding support for versioning the Observable standard library (including recommended libraries) so that authors can opt-in to more recent versions of libraries without retroactively changing the behavior of existing notebooks. I expect you’ll go into the dependencies pane on your notebook to choose which version of the standard library you want.

domoritz commented 1 year ago

Sweet, that all sounds good. I updated the notebook and am almost done updating all my example notebooks to the new client.