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

Add redshift support without changing cli or package.json #2233

Merged
merged 49 commits into from
Feb 3, 2018
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
aa02e58
Add a Redshift dialect that inherits from Postgres.
trevorsmith Jul 11, 2016
8335adb
Turn .index() and .dropIndex() into no-ops with warnings in the Redsh…
trevorsmith Jul 11, 2016
e470351
Update the Redshift dialect to be compatible with master.
trevorsmith Feb 6, 2017
75dc23a
Update package.json
trevorsmith Jul 11, 2016
dd7820c
Disable liftoff cli
trevorsmith Jul 11, 2016
2dc1b2b
Remove the CLI
trevorsmith Jul 11, 2016
5fe08ef
Add lib to the repo
trevorsmith Jul 11, 2016
c932345
Allow the escaping of named bindings.
trevorsmith Jul 12, 2016
7e89c1b
Update dist
trevorsmith Sep 28, 2016
2f2626c
Update the Redshift dialect’s instantiation of the query and column c…
trevorsmith Nov 16, 2016
f6e3bb5
Update the distribution
trevorsmith Feb 6, 2017
7f294a1
Fix a merge conflict
trevorsmith Feb 15, 2017
fad6417
Take lib back out
acofer Sep 20, 2017
5dafccd
Trying to bring back in line with tgreisser/knex
acofer Sep 20, 2017
aea2cc4
Merge branch 'master' into master
acofer Sep 20, 2017
847cf11
Merge pull request #1 from acofer/master
acofer Sep 20, 2017
b807272
Add npm 5 package-lock
acofer Sep 20, 2017
3667d2a
Bring cli.js back in line
acofer Sep 21, 2017
4f542b5
Bring cli.js back in line
acofer Sep 21, 2017
cb5ca92
Merge branch 'master' of github.com:skuid/knex
acofer Sep 21, 2017
207e316
Progress commit on redshift integration tests
acofer Sep 21, 2017
e811206
Revert "Progress commit on redshift integration tests"
acofer Sep 22, 2017
68b9740
Progress commit
acofer Sep 22, 2017
1046848
Working not null on primary columns in createTable
acofer Sep 26, 2017
73db01e
Working redshift unit tests
acofer Sep 26, 2017
9633eb1
Working unit and integration tests, still need to fix migration tests
acofer Sep 27, 2017
001c270
Brought datatypes more in line with what redshift actually supports
acofer Sep 27, 2017
8a480fd
Added query compiler unit tests
acofer Sep 27, 2017
841d670
Add a hacky returning clause for redshift ugh
acofer Sep 27, 2017
5cdb44d
Working migration integration tests
acofer Sep 27, 2017
c4d2bc6
Working insert integration tests
acofer Sep 28, 2017
f0de2a2
Allow multiple insert returning values
acofer Sep 28, 2017
add23b6
Working select integration tests
acofer Sep 28, 2017
4a5cc77
Working join integration tests
acofer Sep 28, 2017
ec66caf
Working aggregate integration tests
acofer Sep 28, 2017
fcf71e5
All integration suite tests working
acofer Sep 29, 2017
d6971b2
Put docker index for reconnect tests back
acofer Oct 2, 2017
918e252
Merge remote-tracking branch 'upstream/master'
acofer Oct 2, 2017
c1b16c0
Redshift does not support insert...returning, there does not seem to …
acofer Oct 4, 2017
a12d757
Merge branch 'master' into master
acofer Oct 4, 2017
b41c5d4
Leave redshift integration tests in place, but do not run them by def…
acofer Oct 11, 2017
ca3c54a
Fix mysql order by test
acofer Oct 11, 2017
eac973d
Fix more tests
acofer Oct 11, 2017
ae21482
Merge branch 'master' into master
acofer Nov 15, 2017
5200d03
Merge remote-tracking branch 'upstream/master'
acofer Nov 15, 2017
095c403
Change test db name to knex_test for consistency
acofer Nov 15, 2017
78a48af
Merge branch 'master' of github.com:skuid/knex
acofer Nov 15, 2017
3887f3f
Merge branch 'master' into master
elhigu Feb 1, 2018
007180a
Address PR comments
acofer Feb 2, 2018
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ module.exports = {
"comma-dangle": 0,
"no-unused-vars": [warning, {"vars": "all", "args": "none"}],
"no-console": warning,
"no-var": 2,
"no-debugger": warning,
"indent": [warning, 2, {"SwitchCase": 1}],
"max-len": [warning, 100, 2],
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ raw
.DS_Store
.vagrant
node_modules
package-lock.json
test.sqlite3
npm-debug.log
tmp
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@
"pre_test": "npm run lint && npm run docker_test",
"tape": "node test/tape/index.js | tap-spec",
"debug_tape": "node-debug test/tape/index.js",
"test": "npm run pre_test && istanbul --config=test/.istanbul.yml cover node_modules/mocha/bin/_mocha -- --check-leaks -t 10000 -b -R spec test/index.js && npm run tape"
"test": "node_modules/mocha/bin/mocha --inspect --check-leaks -t 10000 -b -R spec test/index.js"
Copy link
Member

@elhigu elhigu Sep 22, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why have you removed coverage and tape tests and pre_test script call?

Copy link
Contributor Author

@acofer acofer Sep 22, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just a progress commit. I should have cut a feature branch. I'll ping you when it's ready.

},
"bin": {
"knex": "./bin/cli.js"
Expand Down
2 changes: 1 addition & 1 deletion scripts/docker-for-test.sh
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#!/usr/bin/env bash

if [ -n "$(which docker)" ]; then
DOCKER_IMAGES=("mysql:5.7" "postgres:9.6")
DOCKER_IMAGES=("mysql:5.7" "postgres:9.6" "quay.io/skuid/docker-aws-redshift")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these docker images are for connection breaking etc tests, not used for general travis CI integration tests.

for image in ${DOCKER_IMAGES[@]}; do
if [ -z "$(docker images -q ${image})" ]; then
echo "Installing Docker image ${image}"
Expand Down
2 changes: 1 addition & 1 deletion src/dialects/postgres/query/compiler.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ assign(QueryCompiler_PG.prototype, {
sql += ' and table_schema = ?';
bindings.push(this.single.schema);
} else {
sql += ' and table_schema = current_schema';
sql += ' and table_schema = current_schema()';
}

return {
Expand Down
4 changes: 2 additions & 2 deletions src/dialects/postgres/schema/compiler.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ SchemaCompiler_PG.prototype.hasTable = function(tableName) {
sql += ' and table_schema = ?';
bindings.push(this.schema);
} else {
sql += ' and table_schema = current_schema';
sql += ' and table_schema = current_schema()';
}

this.pushQuery({
Expand All @@ -40,7 +40,7 @@ SchemaCompiler_PG.prototype.hasColumn = function(tableName, columnName) {
sql += ' and table_schema = ?';
bindings.push(this.schema);
} else {
sql += ' and table_schema = current_schema';
sql += ' and table_schema = current_schema()';
}

this.pushQuery({
Expand Down
49 changes: 49 additions & 0 deletions src/dialects/redshift/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@

// Redshift
// -------
import inherits from 'inherits';
import Client_PG from '../postgres';
import { assign } from 'lodash'

import Transaction from './transaction';
import QueryCompiler from './query/compiler';
import ColumnCompiler from './schema/columncompiler';
import TableCompiler from './schema/tablecompiler';
import SchemaCompiler from './schema/compiler';

function Client_Redshift(config) {
Client_PG.apply(this, arguments)
}
inherits(Client_Redshift, Client_PG)

assign(Client_Redshift.prototype, {
transaction() {
return new Transaction(this, ...arguments)
},

queryCompiler() {
return new QueryCompiler(this, ...arguments)
},

columnCompiler() {
return new ColumnCompiler(this, ...arguments)
},

tableCompiler() {
return new TableCompiler(this, ...arguments)
},

schemaCompiler() {
return new SchemaCompiler(this, ...arguments)
},

dialect: 'redshift',

driverName: 'pg',

_driver() {
return require('pg')
},
})

export default Client_Redshift;
25 changes: 25 additions & 0 deletions src/dialects/redshift/query/compiler.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@

// Redshift Query Builder & Compiler
// ------
import inherits from 'inherits';

import QueryCompiler_PG from '../../postgres/query/compiler';

import { assign } from 'lodash'

function QueryCompiler_Redshift(client, builder) {
QueryCompiler_PG.call(this, client, builder);
}
inherits(QueryCompiler_Redshift, QueryCompiler_PG);

assign(QueryCompiler_Redshift.prototype, {
truncate() {
return `truncate ${this.tableName}`;
},

_returning(value) {
return '';
}
})

export default QueryCompiler_Redshift;
38 changes: 38 additions & 0 deletions src/dialects/redshift/schema/columncompiler.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@

// Redshift Column Compiler
// -------

import inherits from 'inherits';
import ColumnCompiler_PG from '../../postgres/schema/columncompiler';

import { assign } from 'lodash'

function ColumnCompiler_Redshift() {
ColumnCompiler_PG.apply(this, arguments);
}
inherits(ColumnCompiler_Redshift, ColumnCompiler_PG);

assign(ColumnCompiler_Redshift.prototype, {
bigincrements: 'bigint identity(1,1) primary key not null',
binary: 'varchar(max)',
bit(column) {
return column.length !== false ? `char(${column.length})` : 'char(1)';
},
blob: 'varchar(max)',
datetime: 'timestamp',
enu: 'text',
enum: 'text',
increments: 'integer identity(1,1) primary key not null',
json: 'varchar(max)',
jsonb: 'varchar(max)',
longblob: 'varchar(max)',
mediumblob: 'varchar(max)',
set: 'text',
text: 'varchar(max)',
timestamp: 'timestamp',
tinyblob: 'text',
uuid: 'char(32)',
varbinary: 'varchar(max)'
})

export default ColumnCompiler_Redshift;
60 changes: 60 additions & 0 deletions src/dialects/redshift/schema/compiler.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/* eslint max-len: 0 */

// Redshift Table Builder & Compiler
// -------

import inherits from 'inherits';
import SchemaCompiler_PG from '../../postgres/schema/compiler';

import { assign } from 'lodash';

function SchemaCompiler_Redshift() {
SchemaCompiler_PG.apply(this, arguments);
}
inherits(SchemaCompiler_Redshift, SchemaCompiler_PG);

assign(SchemaCompiler_Redshift.prototype, {

// dropTableIfExists(tableName) {
// // Nota bene: Redshift actually supports 'IF EXISTS', but the
// // docker container used for integrations tests does not.
// let sql = 'case when (select 1 from information_schema.tables where table_name = ?';
// const bindings = [tableName];
// if (this.schema) {
// sql += ' and table_schema = ?';
// bindings.push(this.schema);
// } else {
// sql += ' and table_schema = current_schema()';
// }
// sql += ') then drop table ?;';
// bindings.push(tableName);
// this.pushQuery({
// sql,
// bindings,
// output(resp) {
// return resp.rows.length > 0;
// }
// });
// },

// Check whether the current table
hasTable(tableName) {
let sql = 'select * from information_schema.tables where table_name = ?';
const bindings = [tableName];
if (this.schema) {
sql += ' and table_schema = ?';
bindings.push(this.schema);
} else {
sql += ' and table_schema = current_schema()';
}
this.pushQuery({
sql,
bindings,
output(resp) {
return resp.rows.length > 0;
}
});
},
});

export default SchemaCompiler_Redshift;
22 changes: 22 additions & 0 deletions src/dialects/redshift/schema/tablecompiler.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/* eslint max-len: 0 */

// Redshift Table Builder & Compiler
// -------

import { warn } from '../../../helpers';
import inherits from 'inherits';
import TableCompiler_PG from '../../postgres/schema/tablecompiler';

function TableCompiler_Redshift() {
TableCompiler_PG.apply(this, arguments);
}
inherits(TableCompiler_Redshift, TableCompiler_PG);

TableCompiler_Redshift.prototype.index = function(columns, indexName, indexType) {
warn('Redshift does not support the creation of indexes.');
};
TableCompiler_Redshift.prototype.dropIndex = function(columns, indexName) {
warn('Redshift does not support the deletion of indexes.');
};

export default TableCompiler_Redshift;
21 changes: 21 additions & 0 deletions src/dialects/redshift/transaction.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@

import Promise from 'bluebird';
import { warn } from '../../helpers';
import Transaction from '../../transaction';

export default class Redshift_Transaction extends Transaction {
savepoint(conn) {
warn('Redshift does not support savepoints.');
return Promise.resolve()
}

release(conn, value) {
warn('Redshift does not support savepoints.');
return Promise.resolve()
}

rollbackTo(conn, error) {
warn('Redshift does not support savepoints.');
return Promise.resolve()
}
}
8 changes: 7 additions & 1 deletion src/migrate/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,15 @@ export default class Migrator {

forceFreeMigrationsLock(config) {
this.config = this.setConfig(config);
console.log("LOOKING FOR LOCKTABLENAME");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

leftover debug prints should be removed

const lockTable = this._getLockTableName();
console.log(`LOOKING FOR HASTABLE(${lockTable}) with ${typeof this.knex.schema.hasTable}`);
debugger;
return this.knex.schema.hasTable(lockTable)
.then(exist => exist && this._freeLock());
.then(exist => {
console.log("LOOKING FOR EXIST");
return exist && this._freeLock();
});
}

// Creates a new migration, with a given name.
Expand Down
1 change: 0 additions & 1 deletion src/raw.js
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,6 @@ function replaceRawArrBindings(raw, formatter) {

function replaceKeyBindings(raw, formatter) {
const values = raw.bindings

let { sql } = raw

const regex = /\\?(:(\w+):(?=::)|:(\w+):(?!:)|:(\w+))/g
Expand Down
3 changes: 2 additions & 1 deletion test/docker/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ if (canRunDockerTests()) {
for (const dialectName in config) {
if (config[dialectName].docker) {
describe(`${dialectName} dialect`, function() {
require('./reconnect')(config[dialectName], knex);
console.log("skipping docker reconnect tests");
// require('./reconnect')(config[dialectName], knex);
})
}
}
Expand Down
5 changes: 5 additions & 0 deletions test/docker/reconnect.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ module.exports = function(config, knex) {
this.timeout(process.env.KNEX_TEST_TIMEOUT || 30000);

before(function () {
console.log("DOCKERRRRRRR");
docker = new Docker();
});

Expand All @@ -40,14 +41,17 @@ module.exports = function(config, knex) {
describe('start container and wait until it is ready', function () {

beforeEach(function () {
console.log("NO THIS ONE");
container = new ContainerClass(docker, dockerConf);
return container.start().then(function () {
console.log("GOT HERE");
return waitReadyForQueries();
});
});

describe('initialize connection pool', function () {
beforeEach(function () {
console.log("POOOOOOOL");
connectionPool = createPool();
});

Expand All @@ -58,6 +62,7 @@ module.exports = function(config, knex) {
describe('stop db-container and expect queries to fail', function () {

beforeEach(function () {
console.log("STOOOOOOOOOP");
return container.stop();
});

Expand Down
Loading