Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PRAGMA must run under query method in Capacitor sqlite #10687

Open
18 tasks
rkreutzer opened this issue Feb 7, 2024 · 14 comments
Open
18 tasks

PRAGMA must run under query method in Capacitor sqlite #10687

rkreutzer opened this issue Feb 7, 2024 · 14 comments

Comments

@rkreutzer
Copy link

rkreutzer commented Feb 7, 2024

Issue description

Pull request 10273 moved PRAGMA requests to the run method in v 0.3.20, and they now fail

Expected Behavior

PRAGMA requests should use the query method in Capacitor sqlite, so they execute correctly. #10273, which was merged in version 0.3.20, moved it to the run method.

The owner of Capacitor sqlite repo has verified this in capacitor-community/sqlite#512. Also, sqlite, sqlite-abstract and better-sqlite3 all execute PRAGMA requests under the query method.

Actual Behavior

Executing PRAGMA table_info(...) in v 0.3.20 fails with a result 100 error. See #10273 (comment) for a screenshot.

Steps to reproduce

Execute "PRAGMA table_info('user')"

My Environment

Dependency Version
Operating System Android & iOS
Node.js version 18.7.1
Typescript version 5.2.2
TypeORM version 0.3.20

Additional Context

No response

Relevant Database Driver(s)

  • aurora-mysql
  • aurora-postgres
  • better-sqlite3
  • cockroachdb
  • cordova
  • expo
  • mongodb
  • mysql
  • nativescript
  • oracle
  • postgres
  • react-native
  • sap
  • spanner
  • sqlite
  • sqlite-abstract
  • sqljs
  • sqlserver

Are you willing to resolve this issue by submitting a Pull Request?

Yes, I have the time, and I know how to start.

@jepiqueau
Copy link

i am the owner of @capacitor-community/sqlite, someone has modified the capacitor driver without understanding it. PRAGMA must be removed from

            else if (["INSERT", "UPDATE", "DELETE", "PRAGMA"].indexOf(command) !== -1) {
                raw = await databaseConnection.run(query, parameters, false);
            }

the right code is

            else if (["INSERT", "UPDATE", "DELETE"].indexOf(command) !== -1) {
                raw = await databaseConnection.run(query, parameters, false);
            }

i someone from the typeOrm team can fix that quickly i will appreciate it

@BARNZ
Copy link

BARNZ commented Mar 13, 2024

Hi @vorujack as per your comment in your MR #10273 could you explain your reasoning for this change for us? It has caused some breaking changes for everyone such as the one outlined in this issue

image

BARNZ pushed a commit to BARNZ/typeorm that referenced this issue Mar 13, 2024
…back to .run() instead of .query() for all PRAGMA calls.
BARNZ pushed a commit to BARNZ/typeorm that referenced this issue Mar 13, 2024
… back to .run() instead of .query() for all PRAGMA calls.
@BARNZ
Copy link

BARNZ commented Mar 13, 2024

Have created a simple MR #10774 to fix this issue by undoing just the breaking changes made in the original MR

@jepiqueau
Copy link

@BARNZ i am the owner of the @capacitor-community/sqlite the method to use for PRAMA is neither run or query it must be execute method to use. This has to be changed in CapacitorQueryRunner and as well in CapacitorDriver files

@jepiqueau
Copy link

@BARNZ this will be added to the doc API.md of @capacitor-community/sqlite

## SQLite Commands Within the Plugin

 - SQLite Data Definition Language commands (such as CREATE, ALTER, DROP) should be executed using the `execute` plugin method.

 - SQLite Transaction Control commands (including BEGIN TRANSACTION, COMMIT, ROLLBACK) should also be executed using the `execute` plugin method.

 - SQLite Data Manipulation Language commands (like INSERT, UPDATE, DELETE, REPLACE) should use the `run` plugin method if they involve bind values. They can utilize either the `execute` or `run` plugin methods if no bind values are involved.

 - SQLite Data Query Language commands (SELECT) should be executed using the `query` plugin method.

 - SQLite Special commands (PRAGMA) should be executed using the `execute` plugin method.

@SaintPepsi
Copy link

SaintPepsi commented Apr 28, 2024

When I Apply the patch to use .execute I get the following error:

Error: Execute: unknown error: Queries cannot be performed using execSQL(), use query() instead.

My CapacitoryQueryRunner.js:

if ([
    "BEGIN",
    "ROLLBACK",
    "COMMIT",
    "CREATE",
    "ALTER",
    "DROP",
    "PRAGMA"
].indexOf(command) !== -1) {
    raw = await databaseConnection.execute(query, false);
}

Any ideas on how to fix this? it's driving me crazy, I don't know enough about SQL to fully dive into the root problem.

Edit:
I'm using await queryRunner.createForeignKeys,

@jepiqueau
Copy link

@SaintPepsi the script i run is

const fs = require('fs');

/* Moddify CapacitorQueryRunner.js */
let filePath = './node_modules/typeorm/driver/capacitor/CapacitorQueryRunner.js';
const correctBugInCapacitorQueryRunner = (file) => {
  if (fs.existsSync(file)) {
    fs.readFile(file, 'utf8', function (err, data) {
        if (err) {
            return console.error(err);
        }

        const index = `"DROP",`
        if (index === -1) {
            console.warn('Line not found. Package probably fixed.');
            return;
        }

        var result = data.replace(
          `    "DROP",`,
          `    "DROP",
               "PRAGMA"`

        );
        result = result.replace(
            'else if (["INSERT", "UPDATE", "DELETE", "PRAGMA"].indexOf(command) !== -1) {',
            'else if (["INSERT", "UPDATE", "DELETE"].indexOf(command) !== -1) {'
        );

        fs.writeFile(file, result, 'utf8', function (err) {
            if (err) return console.error(err);
        });
    });
  } else {
      utils.warn(`Couldn't find file ${file}`);
  }

}
/* Moddify CapacitorDriver.js */
const correctBugInCapacitorDriver = (file) => {
  if (fs.existsSync(file)) {
      fs.readFile(file, 'utf8', function (err, data) {
          if (err) {
              return console.error(err);
          }

          const index = data.indexOf('await connection.run(`PRAGMA foreign_keys = ON`);');
          if (index === -1) {
              console.warn('Line not found. Package probably fixed.');
              return;
          }

          var result = data.replace(
              'await connection.run(`PRAGMA foreign_keys = ON`);',
              'await connection.execute(`PRAGMA foreign_keys = ON`, false);'
          );
          result = result.replace(
              'await connection.run(`PRAGMA journal_mode = ${this.options.journalMode}`);',
              'await connection.execute(`PRAGMA journal_mode = ${this.options.journalMode}`, false);'
          );

          fs.writeFile(file, result, 'utf8', function (err) {
              if (err) return console.error(err);
          });
      });
  } else {
      utils.warn(`Couldn't find file ${file}`);
  }
}
correctBugInCapacitorQueryRunner('./node_modules/typeorm/driver/capacitor/CapacitorQueryRunner.js');
correctBugInCapacitorQueryRunner('./node_modules/typeorm/browser/driver/capacitor/CapacitorQueryRunner.js');
correctBugInCapacitorDriver('./node_modules/typeorm/driver/capacitor/CapacitorDriver.js');
correctBugInCapacitorDriver('./node_modules/typeorm/browser/driver/capacitor/CapacitorDriver.js');


/*
const modTypeOrmCapacitor = (filePath, lineToModify, replacementText) => {
  fs.readFile(filePath, 'utf8', (err, data) => {
    if (err) {
      console.error('Error reading file:', err);
      return;
    }
  
    // Split data by line
    const lines = data.split('\n');
  
    // Modify the specific line
    if (lines.length >= lineToModify) {
      lines[lineToModify - 1] = replacementText; // Line numbers are 1-based
    } else {
      console.error('Line number to modify is out of range.');
      return;
    }
  
    // Join lines back together
    const modifiedData = lines.join('\n');
  
    // Write the modified data back to the file
    fs.writeFile(filePath, modifiedData, 'utf8', (err) => {
      if (err) {
        console.error('Error writing file:', err);
        return;
      }
     
      console.log('File modified successfully.');
    });
  });
}

let filePath = './node_modules/typeorm/driver/capacitor/CapacitorQueryRunner.js';
let lineToModify = 61;
let replacementText = '    else if (["INSERT", "UPDATE", "DELETE"].indexOf(command) !== -1) {';
*/

here is an example of a vite.config.js with react

import legacy from '@vitejs/plugin-legacy'
import react from '@vitejs/plugin-react'
import swc from 'unplugin-swc'; // Support using decorators without explicitly set ``types`` in ``TypeORM``
import { defineConfig } from 'vite'

// https://vitejs.dev/config/
export default defineConfig({
  plugins: [
    react(),
    legacy(),
    swc.vite({
      exclude: [], //Default would exclude all file from ``node_modules``
      jsc: {
        minify: {
          compress: true,
          mangle: true,
          //Suggested by ``capacitor-sqlite``
          keep_classnames: true,
          keep_fnames: true,
        },
      },
    }),

  ],
  test: {
    globals: true,
    environment: 'jsdom',
    setupFiles: './src/setupTests.ts',
  }
})

@SaintPepsi
Copy link

Thanks for posting this @jepiqueau unfortunately still getting the same error with this:

initialiseDatabase.ts:47 QueryFailedError: Execute: unknown error: Queries cannot be performed using execSQL(), use query() instead.
    at CapacitorQueryRunner.query (CapacitorQueryRunner.js:81:19)
    at async Promise.all (introduction/index 0)
    at async AbstractSqliteQueryRunner.js:911:42
    at async Promise.all (introduction/index 0)
    at async CapacitorQueryRunner.getCachedTable (BaseQueryRunner.js:176:15)
    at async CapacitorQueryRunner.createForeignKeys (AbstractSqliteQueryRunner.js:685:15)
    at async MasterSeed0000000000001.up (MasterSeed.ts:117:13)
    at async MigrationExecutor.executePendingMigrations (MigrationExecutor.js:220:46)
    at async DataSource.runMigrations (DataSource.js:263:16)
    at async Zr (initialiseDatabase.ts:33:30)

I'm trying to have a look at the source files but I'm just getting into a dead end where the file i'm trying to look at is just this:
image

I don't understand where I need to be looking 🤔

@SaintPepsi
Copy link

Okay, I just noticed that if I don't use queryRunner.createForeignKeys it works!
Instead I have to add foreignKeys to the new Table options

await queryRunner.createTable(
    new Table({
        name: 'Goal'
        columns: ....
        foreignKeys: [ ... foreign key data ] // <- this works!! 
    })
)

@SaintPepsi
Copy link

SaintPepsi commented Apr 28, 2024

I think the culprite is src/driver/sqlite-abstract/AbstractSqliteQueryRunner.ts at
1367 -> loadPragmaRecords

Because it goes:

  1. AbstractSqliteQueryRunner -> createForeignKeys
  2. await this.getCachedTable(tableOrName) because i'm passing the table name as a string
  3. BaseQueryRunner.ts -> await this.loadTables([tableName])
  4. AbstractSqliteQueryRunner ->
await Promise.all([
    this.loadPragmaRecords(tablePath, `table_xinfo`),
    this.loadPragmaRecords(tablePath, `index_list`),
    this.loadPragmaRecords(tablePath, `foreign_key_list`),
])

AbstractSqliteQueryRunner.ts#L1367-L1369

And because they are all being executed in parallel they fail if i'm correct because in the capacitor driver it's using execute which can't be run in parallel? 🕵️‍♂️

@SaintPepsi
Copy link

SaintPepsi commented Apr 28, 2024

Looks like you can only use queryrunner.query most other methods fail, I had queryRunner.dropTable('Tablename');which caused the same error:

query failed:  PRAGMA table_xinfo("CalendarEntry")
error:  Error: Execute: unknown error: Queries cannot be performed using execSQL(), use query() instead.

So for my migration revert the solution was instead of await queryRunner.dropTable('TableName'); to just use await queryRunner.query('DROP TABLE "TableName"');

@jepiqueau
Copy link

@SaintPepsi i thing for vue the vite.config.js should be something like this even if i have not tested it

import { defineConfig } from 'vite';
import vue from '@vitejs/plugin-vue';
import legacy from '@vitejs/plugin-legacy';
import swc from 'unplugin-swc';
import { resolve } from 'path';

export default defineConfig({
  plugins: [
    vue(),
    legacy(),
    swc.vite({
      exclude: [], // Default would exclude all files from `node_modules`
      jsc: {
        minify: {
          compress: true,
          mangle: true,
          // Suggested by `capacitor-sqlite`
          keep_classnames: true,
          keep_fnames: true,
        },
      },
    }),
  ],
  resolve: {
    alias: {
      '@': resolve(__dirname, 'src'),
    },
  },
});

Can you share your example on github so i can have a look

@jepiqueau
Copy link

@SaintPepsi when you said Also, sqlite, sqlite-abstract and better-sqlite3 all execute PRAGMA requests under the query method. can you show me where i can found example of this. if it is true i may reconsider it and allow PRAGMA on the query method of the capacitor plugin

@SaintPepsi
Copy link

@jepiqueau I'm not quite sure what you mean? are you able to elaborate?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants