joe/kami-parse-server
Public Access
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Postgres: prepend className to unique indexes (#6741)

Browse Source 
* prepend className to unique index to allow multiple unique indexes for different classes

* add testcase

* switched test so it can be tested on older versions of parse-server and show failure

* get rid of console log messages on restart by checking if the index exists before creating it

* add IF NOT EXISTS and IF EXISTS to ALTER TABLE

* revert some of code

* ensureIndex use IF NOT EXISTS

* ALTER TABLE CONSTRAINT can't use IF, ADD/DROP COLUMN can

* retesting

* update

* switchted to CREATE UNIQUE INDEX instrad of ALTER TABLE... ALTER TABLE doesn't seem to be needed
This commit is contained in:
Corey
2020-10-12 01:47:45 -04:00
committed by GitHub
parent 8c88d8197b
commit de7ec58de3
2 changed files with 96 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files

112
spec/PostgresStorageAdapter.spec.js
View File

@@ -9,7 +9,7 @@ const getColumns = (client, className) => {
return client.map( return client.map(
'SELECT column_name FROM information_schema.columns WHERE table_name = $<className>', 'SELECT column_name FROM information_schema.columns WHERE table_name = $<className>',
{ className }, { className },
a => a.column_name (a) => a.column_name
); );
}; };
@@ -25,7 +25,7 @@ describe_only_db('postgres')('PostgresStorageAdapter', () => {
return adapter.deleteAllClasses(); return adapter.deleteAllClasses();
}); });
it('schemaUpgrade, upgrade the database schema when schema changes', done => { it('schemaUpgrade, upgrade the database schema when schema changes', (done) => {
const client = adapter._client; const client = adapter._client;
const className = '_PushStatus'; const className = '_PushStatus';
const schema = { const schema = {
@@ -39,7 +39,7 @@ describe_only_db('postgres')('PostgresStorageAdapter', () => {
adapter adapter
.createTable(className, schema) .createTable(className, schema)
.then(() => getColumns(client, className)) .then(() => getColumns(client, className))
.then(columns => { .then((columns) => {
expect(columns).toContain('pushTime'); expect(columns).toContain('pushTime');
expect(columns).toContain('source'); expect(columns).toContain('source');
expect(columns).toContain('query'); expect(columns).toContain('query');
@@ -49,17 +49,17 @@ describe_only_db('postgres')('PostgresStorageAdapter', () => {
return adapter.schemaUpgrade(className, schema); return adapter.schemaUpgrade(className, schema);
}) })
.then(() => getColumns(client, className)) .then(() => getColumns(client, className))
.then(columns => { .then((columns) => {
expect(columns).toContain('pushTime'); expect(columns).toContain('pushTime');
expect(columns).toContain('source'); expect(columns).toContain('source');
expect(columns).toContain('query'); expect(columns).toContain('query');
expect(columns).toContain('expiration_interval'); expect(columns).toContain('expiration_interval');
done(); done();
}) })
.catch(error => done.fail(error)); .catch((error) => done.fail(error));
}); });
it('schemaUpgrade, maintain correct schema', done => { it('schemaUpgrade, maintain correct schema', (done) => {
const client = adapter._client; const client = adapter._client;
const className = 'Table'; const className = 'Table';
const schema = { const schema = {
@@ -73,7 +73,7 @@ describe_only_db('postgres')('PostgresStorageAdapter', () => {
adapter adapter
.createTable(className, schema) .createTable(className, schema)
.then(() => getColumns(client, className)) .then(() => getColumns(client, className))
.then(columns => { .then((columns) => {
expect(columns).toContain('columnA'); expect(columns).toContain('columnA');
expect(columns).toContain('columnB'); expect(columns).toContain('columnB');
expect(columns).toContain('columnC'); expect(columns).toContain('columnC');
@@ -81,7 +81,7 @@ describe_only_db('postgres')('PostgresStorageAdapter', () => {
return adapter.schemaUpgrade(className, schema); return adapter.schemaUpgrade(className, schema);
}) })
.then(() => getColumns(client, className)) .then(() => getColumns(client, className))
.then(columns => { .then((columns) => {
expect(columns.length).toEqual(3); expect(columns.length).toEqual(3);
expect(columns).toContain('columnA'); expect(columns).toContain('columnA');
expect(columns).toContain('columnB'); expect(columns).toContain('columnB');
@@ -89,16 +89,16 @@ describe_only_db('postgres')('PostgresStorageAdapter', () => {
done(); done();
}) })
.catch(error => done.fail(error)); .catch((error) => done.fail(error));
}); });
it('Create a table without columns and upgrade with columns', done => { it('Create a table without columns and upgrade with columns', (done) => {
const client = adapter._client; const client = adapter._client;
const className = 'EmptyTable'; const className = 'EmptyTable';
dropTable(client, className) dropTable(client, className)
.then(() => adapter.createTable(className, {})) .then(() => adapter.createTable(className, {}))
.then(() => getColumns(client, className)) .then(() => getColumns(client, className))
.then(columns => { .then((columns) => {
expect(columns.length).toBe(0); expect(columns.length).toBe(0);
const newSchema = { const newSchema = {
@@ -111,7 +111,7 @@ describe_only_db('postgres')('PostgresStorageAdapter', () => {
return adapter.schemaUpgrade(className, newSchema); return adapter.schemaUpgrade(className, newSchema);
}) })
.then(() => getColumns(client, className)) .then(() => getColumns(client, className))
.then(columns => { .then((columns) => {
expect(columns.length).toEqual(2); expect(columns.length).toEqual(2);
expect(columns).toContain('columnA'); expect(columns).toContain('columnA');
expect(columns).toContain('columnB'); expect(columns).toContain('columnB');
@@ -176,10 +176,10 @@ describe_only_db('postgres')('PostgresStorageAdapter', () => {
); );
await client await client
.one(analyzedExplainQuery, [tableName, 'objectId', caseInsensitiveData]) .one(analyzedExplainQuery, [tableName, 'objectId', caseInsensitiveData])
.then(explained => { .then((explained) => {
const preIndexPlan = explained; const preIndexPlan = explained;
preIndexPlan['QUERY PLAN'].forEach(element => { preIndexPlan['QUERY PLAN'].forEach((element) => {
//Make sure search returned with only 1 result //Make sure search returned with only 1 result
expect(element.Plan['Actual Rows']).toBe(1); expect(element.Plan['Actual Rows']).toBe(1);
expect(element.Plan['Node Type']).toBe('Seq Scan'); expect(element.Plan['Node Type']).toBe('Seq Scan');
@@ -195,17 +195,17 @@ describe_only_db('postgres')('PostgresStorageAdapter', () => {
'objectId', 'objectId',
caseInsensitiveData, caseInsensitiveData,
]) ])
.then(explained => { .then((explained) => {
const postIndexPlan = explained; const postIndexPlan = explained;
postIndexPlan['QUERY PLAN'].forEach(element => { postIndexPlan['QUERY PLAN'].forEach((element) => {
//Make sure search returned with only 1 result //Make sure search returned with only 1 result
expect(element.Plan['Actual Rows']).toBe(1); expect(element.Plan['Actual Rows']).toBe(1);
//Should not be a sequential scan //Should not be a sequential scan
expect(element.Plan['Node Type']).not.toContain('Seq Scan'); expect(element.Plan['Node Type']).not.toContain('Seq Scan');
//Should be using the index created for this //Should be using the index created for this
element.Plan.Plans.forEach(innerElement => { element.Plan.Plans.forEach((innerElement) => {
expect(innerElement['Index Name']).toBe(indexName); expect(innerElement['Index Name']).toBe(indexName);
}); });
}); });
@@ -230,8 +230,8 @@ describe_only_db('postgres')('PostgresStorageAdapter', () => {
'objectId', 'objectId',
caseInsensitiveData, caseInsensitiveData,
]) ])
.then(explained => { .then((explained) => {
explained['QUERY PLAN'].forEach(element => { explained['QUERY PLAN'].forEach((element) => {
//Check that basic query plans isn't a sequential scan //Check that basic query plans isn't a sequential scan
expect(element.Plan['Node Type']).not.toContain( expect(element.Plan['Node Type']).not.toContain(
'Seq Scan' 'Seq Scan'
@@ -244,7 +244,7 @@ describe_only_db('postgres')('PostgresStorageAdapter', () => {
}); });
}); });
}) })
.catch(error => { .catch((error) => {
// Query on non existing table, don't crash // Query on non existing table, don't crash
if (error.code !== '42P01') { if (error.code !== '42P01') {
throw error; throw error;
@@ -276,8 +276,8 @@ describe_only_db('postgres')('PostgresStorageAdapter', () => {
{ caseInsensitive: true, explain: true } { caseInsensitive: true, explain: true }
); );
preIndexPlan.forEach(element => { preIndexPlan.forEach((element) => {
element['QUERY PLAN'].forEach(innerElement => { element['QUERY PLAN'].forEach((innerElement) => {
//Check that basic query plans isn't a sequential scan, be careful as find uses "any" to query //Check that basic query plans isn't a sequential scan, be careful as find uses "any" to query
expect(innerElement.Plan['Node Type']).toBe('Seq Scan'); expect(innerElement.Plan['Node Type']).toBe('Seq Scan');
//Basic query plans shouldn't have an execution time //Basic query plans shouldn't have an execution time
@@ -302,8 +302,8 @@ describe_only_db('postgres')('PostgresStorageAdapter', () => {
{ caseInsensitive: true, explain: true } { caseInsensitive: true, explain: true }
); );
postIndexPlan.forEach(element => { postIndexPlan.forEach((element) => {
element['QUERY PLAN'].forEach(innerElement => { element['QUERY PLAN'].forEach((innerElement) => {
//Check that basic query plans isn't a sequential scan //Check that basic query plans isn't a sequential scan
expect(innerElement.Plan['Node Type']).not.toContain('Seq Scan'); expect(innerElement.Plan['Node Type']).not.toContain('Seq Scan');
@@ -339,13 +339,73 @@ describe_only_db('postgres')('PostgresStorageAdapter', () => {
{ username: caseInsensitiveData }, { username: caseInsensitiveData },
{ caseInsensitive: true, explain: true } { caseInsensitive: true, explain: true }
); );
indexPlan.forEach(element => { indexPlan.forEach((element) => {
element['QUERY PLAN'].forEach(innerElement => { element['QUERY PLAN'].forEach((innerElement) => {
expect(innerElement.Plan['Node Type']).not.toContain('Seq Scan'); expect(innerElement.Plan['Node Type']).not.toContain('Seq Scan');
expect(innerElement.Plan['Index Name']).toContain('parse_default'); expect(innerElement.Plan['Index Name']).toContain('parse_default');
}); });
}); });
}); });
it('should allow multiple unique indexes for same field name and different class', async () => {
const firstTableName = 'Test1';
const firstTableSchema = new Parse.Schema(firstTableName);
const uniqueField = 'uuid';
firstTableSchema.addString(uniqueField);
await firstTableSchema.save();
await firstTableSchema.get();
const secondTableName = 'Test2';
const secondTableSchema = new Parse.Schema(secondTableName);
secondTableSchema.addString(uniqueField);
await secondTableSchema.save();
await secondTableSchema.get();
const database = Config.get(Parse.applicationId).database;
//Create index before data is inserted
await adapter.ensureUniqueness(firstTableName, firstTableSchema, [
uniqueField,
]);
await adapter.ensureUniqueness(secondTableName, secondTableSchema, [
uniqueField,
]);
//Postgres won't take advantage of the index until it has a lot of records because sequential is faster for small db's
const client = adapter._client;
await client.none(
'INSERT INTO $1:name ($2:name, $3:name) SELECT MD5(random()::text), MD5(random()::text) FROM generate_series(1,5000)',
[firstTableName, 'objectId', uniqueField]
);
await client.none(
'INSERT INTO $1:name ($2:name, $3:name) SELECT MD5(random()::text), MD5(random()::text) FROM generate_series(1,5000)',
[secondTableName, 'objectId', uniqueField]
);
//Check using find method for Parse
const indexPlan = await database.find(
firstTableName,
{ uuid: '1234' },
{ caseInsensitive: false, explain: true }
);
indexPlan.forEach((element) => {
element['QUERY PLAN'].forEach((innerElement) => {
expect(innerElement.Plan['Node Type']).not.toContain('Seq Scan');
expect(innerElement.Plan['Index Name']).toContain(uniqueField);
});
});
const indexPlan2 = await database.find(
secondTableName,
{ uuid: '1234' },
{ caseInsensitive: false, explain: true }
);
indexPlan2.forEach((element) => {
element['QUERY PLAN'].forEach((innerElement) => {
expect(innerElement.Plan['Node Type']).not.toContain('Seq Scan');
expect(innerElement.Plan['Index Name']).toContain(uniqueField);
});
});
});
}); });
describe_only_db('postgres')('PostgresStorageAdapter shutdown', () => { describe_only_db('postgres')('PostgresStorageAdapter shutdown', () => {

19
src/Adapters/Storage/Postgres/PostgresStorageAdapter.js
View File

@@ -1128,7 +1128,7 @@ export class PostgresStorageAdapter implements StorageAdapter {
if (type.type !== 'Relation') { if (type.type !== 'Relation') {
try { try {
await t.none( await t.none(
'ALTER TABLE $<className:name> ADD COLUMN $<fieldName:name> $<postgresType:raw>', 'ALTER TABLE $<className:name> ADD COLUMN IF NOT EXISTS $<fieldName:name> $<postgresType:raw>',
{ {
className, className,
fieldName, fieldName,
@@ -1271,7 +1271,10 @@ export class PostgresStorageAdapter implements StorageAdapter {
{ schema, className } { schema, className }
); );
if (values.length > 1) { if (values.length > 1) {
await t.none(`ALTER TABLE $1:name DROP COLUMN ${columns}`, values); await t.none(
`ALTER TABLE $1:name DROP COLUMN IF EXISTS ${columns}`,
values
);
} }
}); });
} }
@@ -2063,13 +2066,11 @@ export class PostgresStorageAdapter implements StorageAdapter {
schema: SchemaType, schema: SchemaType,
fieldNames: string[] fieldNames: string[]
) { ) {
// Use the same name for every ensureUniqueness attempt, because postgres const constraintName = `${className}_unique_${fieldNames.sort().join('_')}`;
// Will happily create the same index with multiple names.
const constraintName = `unique_${fieldNames.sort().join('_')}`;
const constraintPatterns = fieldNames.map( const constraintPatterns = fieldNames.map(
(fieldName, index) => `$${index + 3}:name` (fieldName, index) => `$${index + 3}:name`
); );
const qs = `ALTER TABLE $1:name ADD CONSTRAINT $2:name UNIQUE (${constraintPatterns.join()})`; const qs = `CREATE UNIQUE INDEX IF NOT EXISTS $2:name ON $1:name(${constraintPatterns.join()})`;
return this._client return this._client
.none(qs, [className, constraintName, ...fieldNames]) .none(qs, [className, constraintName, ...fieldNames])
.catch(error => { .catch(error => {
@@ -2491,7 +2492,7 @@ export class PostgresStorageAdapter implements StorageAdapter {
return (conn || this._client).tx(t => return (conn || this._client).tx(t =>
t.batch( t.batch(
indexes.map(i => { indexes.map(i => {
return t.none('CREATE INDEX $1:name ON $2:name ($3:name)', [ return t.none('CREATE INDEX IF NOT EXISTS $1:name ON $2:name ($3:name)', [
i.name, i.name,
className, className,
i.key, i.key,
@@ -2509,7 +2510,7 @@ export class PostgresStorageAdapter implements StorageAdapter {
): Promise<void> { ): Promise<void> {
await ( await (
conn || this._client conn || this._client
).none('CREATE INDEX $1:name ON $2:name ($3:name)', [ ).none('CREATE INDEX IF NOT EXISTS $1:name ON $2:name ($3:name)', [
fieldName, fieldName,
className, className,
type, type,
@@ -2588,7 +2589,7 @@ export class PostgresStorageAdapter implements StorageAdapter {
(fieldName, index) => `lower($${index + 3}:name) varchar_pattern_ops` (fieldName, index) => `lower($${index + 3}:name) varchar_pattern_ops`
) )
: fieldNames.map((fieldName, index) => `$${index + 3}:name`); : fieldNames.map((fieldName, index) => `$${index + 3}:name`);
const qs = `CREATE INDEX $1:name ON $2:name (${constraintPatterns.join()})`; const qs = `CREATE INDEX IF NOT EXISTS $1:name ON $2:name (${constraintPatterns.join()})`;
await conn await conn
.none(qs, [indexNameOptions.name, className, ...fieldNames]) .none(qs, [indexNameOptions.name, className, ...fieldNames])
.catch(error => { .catch(error => {