[email protected] depends on arrow11

[Fil]

[email protected] depends on arrow11 (after duckdb/duckdb-wasm@18dd6c7); because of this, with #356 loadDataSource does not create tables from arrays any more.

In this tentative PR I add full support for arrow11 on top of the existing arrow9 and arrow4. It might be enough to support arrow11 only in the case of duckdb, but supporting it as a filetype felt the right thing to do. (Not tested though.)

DEMO: https://observablehq.com/@observablehq/duckdb-1-24-breaks-insertion-in-sql-mode

Here's a unit test I wanted to add, unfortunately it fails because loadArrow() wants an http-resource.

--- a/test/query-test.js
+++ b/test/query-test.js
@@ -1,5 +1,5 @@
 import assert from "assert";
-import {__query} from "../src/table.js";
+import {__query, loadDataSource} from "../src/table.js";
 import it, {invalidator} from "./invalidation.js";

@@ -109,3 +109,12 @@ describe("__query.sql", () => {
+
+describe("loadDataSource", () => {
+  it("loads an array as a sql table", async () => {
+    const [invalidation] = invalidator();
+    const data = [{x: 1}, {x: 2}];
+    const db = await loadDataSource(data, "sql", "data");
+    assert.strictEqual(await __query.sql(db, invalidation)`SELECT * FROM data`, data); // TODO
+  });
+});

[Fil]

Suggested change

	// Returns true if the vaue is an Apache Arrow table. This uses a “duck” test
	// Returns true if the value is an Apache Arrow table. This uses a “duck” test

[mootari]

Listed below are the JS related changes in the latest apache-arrow releases.

10.0.0:

• ARROW-17321 - [JS] Update dependencies (#13758)
• ARROW-17410 - [JS][Integration] Downgrade zlib for integration (#13885)
• ARROW-17903 - [JS] Update dependencies (#14285)

10.0.1:

• ARROW-18335 - [CI][Release][JS] verify-rc-source-js is failed with Node.js 18

11.0.0:

• ARROW-18233 - [Release][JS] don’t install yarn to system (#14577)
• ARROW-18247 - [JS] fix: RangeError crash in Vector.toArray() (#14587)
• GH-14791 - [JS] Fix BitmapBufferBuilder size truncation (#14881)
• GH-15156 - [JS] Fix can’t find variable: BigInt64Array (#15157)

[annie]

tested locally and this looks good from a Data Table & SQL cell perspective. i'm not sure what else needs to be tested though – what parts are you unsure about @Fil?

[Fil]

The code works in all my tests too— the parts that I'm not sure of are:

1. case 11: { … }: where would that 11 come from? A fileAttachement.arrow({version: 11}) call but on what file? I haven't been able to test this, and not sure it does anything.

Testing now… I have an (old) arrow file on my hard drive, adding it as a FileAttachment and calling:

• flights10k = FileAttachment("flights-10k.arrow").arrow({ version: 11 })
• flights10k = FileAttachment("flights-10k.arrow").arrow({ version: 9 })

both seem to return the same thing. The only difference I can spot is (of course) that they have different prototypes, because they're coming from different code bases.

Maybe using arrow11 for version: 9 works too, and we could remove the dependency on version 9. We would need to just default case 9 to version 11.

2. How to create unit tests so that this does not happen again (see the tentative test in the issue description)

[mbostock]

Maybe using arrow11 for version: 9 works too, and we could remove the dependency on version 9.

Please don’t do this; it breaks backwards compatibility. Just add Arrow 11 as a new dependency.

[annie]

re: tests – i'm working on adding an integration test for arrays in a SQL cell. that should catch any issues at the point where we upgrade stdlib in the monorepo, although a unit test here would catch it earlier, so that would be great too.

[Fil]

OK. So I guess that standing question is, is there any added value of adding {version: 11} as an option to FileAttachment.arrow()? It felt "wrong" not to add it (in the sense that we would be using 11 internally, but not offer it as an option), but I don't know what concrete difference it makes.

[mbostock]

Yes, we should add it; it is needed by the compiler so we’ll need a corresponding PR over there.

[annie]

are we good to re-upgrade stdlib in the monorepo, or do we need the compiler update to go in first?

[mbostock]

Upgrade stdlib first to add Arrow 11, then followup with the compiler update to use it. 👍

[annie]

@mbostock sorry, could you say more about what we need to update in the compiler? i tested this upgrade locally and SQL cells seem to be working as expected. FileAttachment.arrow({version: 11}) works too. is there something else that needs to be updated?

[mbostock]

@annie My mistake. I was thinking of chart cells, but we don’t support Arrow tables as data sources for chart cells yet, so no compiler changes are necessary. This change should stand on its own! 👍

[annie]

ok awesome, thanks for clarifying!