tablelandnetwork / js-tableland-cli

Development has moved to: https://github.com/tablelandnetwork/tableland-js/
MIT License
12 stars 2 forks source link

[JSTC-34] Insert + flat select fails if primary key not specified #392

Closed dtbuchholz closed 1 year ago

dtbuchholz commented 1 year ago

Summary

I have a main table (including an id INTEGER PRIMARY KEY, i.e., auto-incrementing) and an admin table. The main stores misc. data, and admin stores provisioned admin addresses. Only if a value exists in admin can an insert happen with main. I would like to conditionally insert into main, but the parser fails if I try to make an INSERT with a flat SELECT without specifying the id column. In other words, id is required, but since I want to auto-increment this column, this is not ideal.

Steps to reproduce

I have done the following table setup using the CLI with LT:

From here, I want to lookup a value (0xabcd) in admin and only insert data (e.g., a 'test' string) in main if the admin value exists. I cannot do that with the following query, which breaks my desire to auto-increment main:

Instead, I have to specify the id column for main—this does work but again, isn't ideal:

Error

For context, upon trying to insert into main without the id column, the following is logged via the CLI and complains about the main table having 2 columns but only 1 value was supplied:

Error: ALL_ERROR: db query execution failed (code: SQLITE_table main_31337_6 has 2 columns but 1 values were supplied, msg: table main_31337_6 has 2 columns but 1 values were supplied)
    at errorWithCause (file:///Users/dtb/.nvm/versions/node/v18.12.1/lib/node_modules/@tableland/cli/node_modules/@tableland/sdk/dist/esm/lowlevel.js:103:12)
    at Statement.all (file:///Users/dtb/.nvm/versions/node/v18.12.1/lib/node_modules/@tableland/cli/node_modules/@tableland/sdk/dist/esm/statement.js:90:19)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async fireFullQuery (file:///Users/dtb/.nvm/versions/node/v18.12.1/lib/node_modules/@tableland/cli/dist/commands/shell.js:46:30)
    at async shellYeah (file:///Users/dtb/.nvm/versions/node/v18.12.1/lib/node_modules/@tableland/cli/dist/commands/shell.js:113:17) {
  [cause]: Error: db query execution failed (code: SQLITE_table main_31337_6 has 2 columns but 1 values were supplied, msg: table main_31337_6 has 2 columns but 1 values were supplied)
      at errorWithHint (file:///Users/dtb/.nvm/versions/node/v18.12.1/lib/node_modules/@tableland/cli/node_modules/@tableland/sdk/dist/esm/lowlevel.js:119:16)
      at Statement.all (file:///Users/dtb/.nvm/versions/node/v18.12.1/lib/node_modules/@tableland/cli/node_modules/@tableland/sdk/dist/esm/statement.js:89:26)
      at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
      at async fireFullQuery (file:///Users/dtb/.nvm/versions/node/v18.12.1/lib/node_modules/@tableland/cli/dist/commands/shell.js:46:30)
      at async shellYeah (file:///Users/dtb/.nvm/versions/node/v18.12.1/lib/node_modules/@tableland/cli/dist/commands/shell.js:113:17) {
    [cause]: Error: db query execution failed (code: SQLITE_table main_31337_6 has 2 columns but 1 values were supplied, msg: table main_31337_6 has 2 columns but 1 values were supplied)
        at Object.wait (file:///Users/dtb/.nvm/versions/node/v18.12.1/lib/node_modules/@tableland/cli/node_modules/@tableland/sdk/dist/esm/registry/utils.js:50:19)
        at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
        at async checkWait (file:///Users/dtb/.nvm/versions/node/v18.12.1/lib/node_modules/@tableland/cli/node_modules/@tableland/sdk/dist/esm/helpers/config.js:5:24)
        at async Statement._Statement_waitExec (file:///Users/dtb/.nvm/versions/node/v18.12.1/lib/node_modules/@tableland/cli/node_modules/@tableland/sdk/dist/esm/statement.js:196:12)
        at async Statement.all (file:///Users/dtb/.nvm/versions/node/v18.12.1/lib/node_modules/@tableland/cli/node_modules/@tableland/sdk/dist/esm/statement.js:84:39)
        at async fireFullQuery (file:///Users/dtb/.nvm/versions/node/v18.12.1/lib/node_modules/@tableland/cli/dist/commands/shell.js:46:30)
        at async shellYeah (file:///Users/dtb/.nvm/versions/node/v18.12.1/lib/node_modules/@tableland/cli/dist/commands/shell.js:113:17)
  }

Expectation

I'd expect that since I can typically make a query that does not specify the id column, such as doing insert into main_31337_2 (data) values ('test')on its own, it should also work with a INSERT + flat SELECT.

From SyncLinear.com | JSTC-34

dtbuchholz commented 1 year ago

Here is a better example of the type of query we don't want to allow:

with ins as (
    insert into table1 (userid, email)
    values(12,'example@gmail.com') 
)
insert into table2 (userid, name, address)
values(12, 'Joe', '123 test Dr')

This is Postgres, but you get the idea.

dtbuchholz commented 1 year ago

I think the issue is that we have setup checks to ensure a single statement string doesn't mutate two tables. Off the top of my head I think there's two places you'll need to revisit the logic of these checks. Here:

https://github.com/tablelandnetwork/js-tableland/blob/231bb55f8f16b5ff65920bbbad52e62524882a45/src/database.ts#L237

and here:

https://github.com/tablelandnetwork/js-tableland-cli/blob/e7fddc2db6a7253e5ef7df8b7988f8feceaae96b/src/commands/write.ts#L103

If I'm thinking about this right, which might not be the case, we want to allow insert into ${tableMain} (data) select 'test' from ${tableAdmin} where address = '0xabcd';, but not allow insert into ${tableMain} (data) values (1);insert into ${tableMain} (address) values ('0xabcd');

dtbuchholz commented 1 year ago

@joewagner thinking about this a bit more, i think the issue itself actually stems from db.batch(), not the CLI's usage of it. e.g., the following will throw, which is what's bubbling up in the CLI:

// Configure a `signer` to then pass to the `Database` instance
const db = new Database({ signer });

// Create tables
const { meta: createMain } = await db
  .prepare("create table main (id integer primary key, data text);")
  .run();
await createMain.txn?.wait();
const tableMain = createMain.txn?.name;
const { meta: createAdmin } = await db
  .prepare("create table admin (id integer primary key, address text);")
  .run();
await createAdmin.txn?.wait();
const tableAdmin = createAdmin.txn?.name;

// Insert into them
const { meta: insertAdmin } = await db
  .prepare(`insert into ${tableAdmin} (address) values ('0xabcd');`)
  .run();
await insertAdmin.txn?.wait();
const stmt = db.prepare(
  `insert into ${tableMain} (data) select 'test' from ${tableAdmin} where address = '0xabcd';`
);
const insertMain = await db.batch([stmt]);
// ^ error here: Error: BATCH_ERROR: each statement can only touch one table. try batching statements based on the table they mutate.
await insertMain.txn?.wait();

// Get results
const { results } = await db.prepare(`select * from ${tableMain};`).run();
console.log(results);

that is, the prepare method works fine here, but batch does not allow insert into ${tableMain} (data) select 'test' from ${tableAdmin} where address = '0xabcd';. i should be able to batch that statement, right?

dtbuchholz commented 1 year ago

Sounds good to me, let me know if you have any trouble.

dtbuchholz commented 1 year ago

okay gotcha. cool, i'll tal and try to fix if that works for you

dtbuchholz commented 1 year ago

@dtbuchholz I can reproduce this, and confirm it's specifically an issue with the CLI.

It looks like the logic that the CLI is using to batch write statements is throwing because it thinks a single write statement is trying to mutate more than one table.

dtbuchholz commented 1 year ago

I'll try to reproduce this today

dtbuchholz commented 1 year ago

@joewagner lmk if you can recreate this issue. happy to try and debug/fix this myself (and take the issue), but i want to first make sure it's not my machine alone.

dtbuchholz commented 1 year ago

hmm that's not it, but it is the CLI…i'll move this issue there. i tried this setup with the SDK and it worked as expected, so this must be a CLI issue.

for reference, i'm using CLI v5.2.2 (latest), which uses wasm-sqlparser v1.2.1 (latest)—and js-tableland also uses this parser version. and the wasm-sqlparser is using go-sqlparser v0.0.0-20230605164749-c0e6862c37f6 (which also looks up to date).

dtbuchholz commented 1 year ago

I could not replicate this issue by creating a test on go-tabeland. Could be the case that the CLI is outdated and not using the latest parser version.

dtbuchholz commented 1 year ago

Note: I know there are some intricacies with INSERT + SELECT…I'm unsure if this is one of them or a bug. also cc @avichalp