fix(oracle-driver): Release connection after query execution (#5469)

Co-authored-by: a1tair6 <[email protected]>
Co-authored-by: p-eye <[email protected]>
Co-authored-by: Dmitry Patsura <[email protected]>

Author: 4 people

.github/workflows/push.yml

@@ -479,6 +479,8 @@ jobs:
           retry_wait_seconds: 15
           timeout_minutes: 20
           command: yarn install --frozen-lockfile
+      - name: Install instant client for Oracle
+        uses: GoodManWEN/oracle-client-action@main
       - name: Build client
         run: yarn build
       - name: Lerna tsc

packages/cubejs-oracle-driver/driver/OracleDriver.js

@@ -69,7 +69,7 @@ class OracleDriver extends BaseDriver {
     this.db.partRows = 100000;
     this.db.maxRows = 100000;
     this.db.prefetchRows = 500;
-    this.config = config || {
+    this.config = {
       user: getEnv('dbUser', { dataSource }),
       password: getEnv('dbPass', { dataSource }),
       db: getEnv('dbName', { dataSource }),
@@ -80,11 +80,9 @@ class OracleDriver extends BaseDriver {
         config.maxPoolSize ||
         getEnv('dbMaxPoolSize', { dataSource }) ||
         50,
+      ...config
     };
-
-    if (!this.config.connectionString) {
-      this.config.connectionString = `${this.config.host}/${this.config.db}`
-    }
+    this.config.connectionString = this.config.connectionString || `${this.config.host}:${this.config.port}/${this.config.db}`;
   }
 
   async tablesSchema() {
@@ -113,22 +111,28 @@ class OracleDriver extends BaseDriver {
     if (!this.pool) {
       this.pool = await this.db.createPool(this.config);
     }
+
     return this.pool.getConnection()
   }
 
   async testConnection() {
-    return (
-      await this.getConnectionFromPool()
-    ).execute('SELECT 1 FROM DUAL');
+    await this.query('SELECT 1 FROM DUAL', {});
   }
 
   async query(query, values) {
     const conn = await this.getConnectionFromPool();
+
     try {
       const res = await conn.execute(query, values || {});
       return res && res.rows;
     } catch (e) {
       throw (e);
+    } finally {
+      try {
+        await conn.close();
+      } catch (e) {
+        throw e;
+      }
     }
   }
 

packages/cubejs-testing-shared/src/db/index.ts

@@ -7,3 +7,4 @@ export * from './crate';
 export * from './prestodb';
 export * from './mssql';
 export * from './trino';
+export * from './oracle';

packages/cubejs-testing-shared/src/db/oracle.ts

@@ -0,0 +1,26 @@
+import { GenericContainer } from 'testcontainers';
+import { DbRunnerAbstract, DBRunnerContainerOptions } from './db-runner.abstract';
+
+type OracleStartOptions = DBRunnerContainerOptions & {
+  version?: string,
+};
+
+export class OracleDBRunner extends DbRunnerAbstract {
+  public static startContainer(options: OracleStartOptions) {
+    const version = process.env.TEST_ORACLE_VERSION || options.version || '21.3.0';
+
+    const container = new GenericContainer(`gvenzl/oracle-xe:${version}`)
+      .withEnv('ORACLE_PASSWORD', 'test')
+      .withExposedPorts(1521)
+      .withStartupTimeout(30 * 1000);
+
+    if (options.volumes) {
+      // eslint-disable-next-line no-restricted-syntax
+      for (const { source, target, bindMode } of options.volumes) {
+        container.withBindMount(source, target, bindMode);
+      }
+    }
+
+    return container.start();
+  }
+}

packages/cubejs-testing/birdbox-fixtures/oracle/schema/Orders.js

@@ -0,0 +1,30 @@
+cube('Orders', {
+  sql: `
+    select 1 as id, 100 as amount, 'new' status from dual
+    UNION ALL
+    select 2 as id, 200 as amount, 'new' status from dual
+    UNION ALL
+    select 3 as id, 300 as amount, 'processed' status from dual
+    UNION ALL
+    select 4 as id, 500 as amount, 'processed' status from dual
+    UNION ALL
+    select 5 as id, 600 as amount, 'shipped' status from dual
+    `,
+  measures: {
+    totalAmount: {
+      sql: 'amount',
+      type: 'sum',
+    },
+  },
+  dimensions: {
+    id: {
+      sql: 'id',
+      type: 'number'
+    },
+
+    status: {
+      sql: 'status',
+      type: 'string',
+    },
+  }
+});

packages/cubejs-testing/package.json

@@ -48,7 +48,7 @@
     "driver:databricks:snap": "jest --verbose --updateSnapshot -i dist/test/driver-databricks.test.js",
     "integration:cubestore": "jest --verbose --updateSnapshot -i dist/test/driver-cubestore.test.js",
     "rest:postgres": "yarn tsc && clear && jest --verbose -i dist/test/rest-postgres.test.js",
-    "smoke": "jest --verbose -i 'dist/test/smoke-(?!.*?(redshift|bigquery|firebolt|cubesql))'",
+    "smoke": "jest --verbose -i 'dist/test/smoke-(?!.*?(redshift|athena|bigquery|firebolt|cubesql|oracle))'",
     "smoke:athena": "jest --verbose -i dist/test/smoke-athena.test.js",
     "smoke:athena:snapshot": "jest --verbose --updateSnapshot -i dist/test/smoke-athena.test.js",
     "smoke:bigquery": "jest --verbose -i dist/test/smoke-bigquery.test.js",
@@ -61,6 +61,8 @@
     "smoke:lambda:snapshot": "jest --updateSnapshot --verbose -i dist/test/smoke-lambda.test.js",
     "smoke:materialize": "jest --verbose -i dist/test/smoke-materialize.test.js",
     "smoke:materialize:snapshot": "jest --verbose --updateSnapshot -i dist/test/smoke-materialize.test.js",
+    "smoke:oracle": "jest --verbose -i dist/test/smoke-oracle.test.js",
+    "smoke:oracle:snapshot": "jest --verbose --updateSnapshot -i dist/test/smoke-oracle.test.js",
     "smoke:questdb": "jest --verbose -i dist/test/smoke-questdb.test.js",
     "smoke:questdb:snapshot": "jest --verbose --updateSnapshot -i dist/test/smoke-questdb.test.js",
     "smoke:multidb": "jest --verbose -i dist/test/smoke-multidb.test.js",

packages/cubejs-testing/src/REQUIRED_ENV_VARS.ts

@@ -24,6 +24,7 @@ export const REQUIRED_ENV_VARS: {[key: string]: string[]} = {
   ],
   materialize: [],
   multidb: [],
+  oracle: [],
   questdb: [],
   postgres: [],
   redshift: [

packages/cubejs-testing/src/birdbox.ts

@@ -48,7 +48,7 @@ interface Args {
   log: Log,
 }
 
-export type DriverType = 'postgresql' | 'postgres' | 'multidb' | 'materialize' | 'crate' | 'bigquery' | 'athena' | 'postgresql-cubestore' | 'firebolt' | 'questdb' | 'redshift' | 'databricks-jdbc' | 'prestodb' | 'mssql' | 'trino';
+export type DriverType = 'postgresql' | 'postgres' | 'multidb' | 'materialize' | 'crate' | 'bigquery' | 'athena' | 'postgresql-cubestore' | 'firebolt' | 'questdb' | 'redshift' | 'databricks-jdbc' | 'prestodb' | 'mssql' | 'trino' | 'oracle';
 
 export type Schemas = string[];
 
@@ -105,7 +105,8 @@ const driverNameToFolderNameMapper: Record<DriverType, string> = {
   'databricks-jdbc': 'databricks-jdbc',
   prestodb: 'postgresql',
   mssql: 'mssql',
-  trino: 'postgresql'
+  trino: 'postgresql',
+  oracle: 'oracle'
 };
 
 /**

packages/cubejs-testing/test/__snapshots__/smoke-oracle.test.ts.snap

@@ -0,0 +1,9 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`oracle query measure: query 1`] = `
+Array [
+  Object {
+    "Orders.totalAmount": 1700,
+  },
+]
+`;

packages/cubejs-testing/test/smoke-oracle.test.ts

@@ -0,0 +1,46 @@
+import { StartedTestContainer } from 'testcontainers';
+import { OracleDBRunner } from '@cubejs-backend/testing-shared';
+import cubejs, { CubejsApi } from '@cubejs-client/core';
+// eslint-disable-next-line import/no-extraneous-dependencies
+import { afterAll, beforeAll, expect, jest } from '@jest/globals';
+import { pausePromise } from '@cubejs-backend/shared';
+import { BirdBox, getBirdbox } from '../src';
+import { DEFAULT_CONFIG, testQueryMeasure } from './smoke-tests';
+
+describe('oracle', () => {
+  jest.setTimeout(60 * 5 * 100000);
+  let db: StartedTestContainer;
+  let birdbox: BirdBox;
+  let client: CubejsApi;
+
+  beforeAll(async () => {
+    db = await OracleDBRunner.startContainer({});
+    birdbox = await getBirdbox(
+      'oracle',
+      {
+        CUBEJS_DB_TYPE: 'oracle',
+
+        CUBEJS_DB_HOST: db.getHost(),
+        CUBEJS_DB_PORT: `${db.getMappedPort(1521)}`,
+        CUBEJS_DB_NAME: 'XE',
+        CUBEJS_DB_USER: 'system',
+        CUBEJS_DB_PASS: 'test',
+
+        ...DEFAULT_CONFIG,
+      },
+      {
+        schemaDir: 'oracle/schema',
+      }
+    );
+    client = cubejs(async () => 'test', {
+      apiUrl: birdbox.configuration.apiUrl,
+    });
+  });
+
+  afterAll(async () => {
+    await birdbox.stop();
+    await db.stop();
+  });
+
+  test('query measure', () => testQueryMeasure(client));
+});