Skip to content

Commit

Permalink
fix(resource): ensure that redundant aliases and join wrapping parens…
Browse files Browse the repository at this point in the history
… dont affect diff for VIEW

also, refactor the heavily normalize view dir for better maintainability
  • Loading branch information
uladkasach committed Apr 12, 2020
1 parent b954d9f commit f6e9f7f
Show file tree
Hide file tree
Showing 19 changed files with 361 additions and 88 deletions.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -137,4 +137,81 @@ CREATE VIEW view_spaceship_with_cargo AS
});
expect(normalizedShowCreateDefSql).toEqual(normalizedUserSqlDef);
});
it('should find no change on this real world example where recursive support for views is required', async () => {
// define the view sql
const userDefSql = `
CREATE VIEW \`view_contractor_current\` AS
SELECT
s.id,
s.uuid,
s.name,
(
SELECT GROUP_CONCAT(contractor_version_to_contractor_license.contractor_license_id ORDER BY contractor_version_to_contractor_license.array_order_index asc separator ',')
FROM contractor_version_to_contractor_license WHERE contractor_version_to_contractor_license.contractor_version_id = v.id
) as license_ids,
(
SELECT GROUP_CONCAT(contractor_version_to_contact_method.contact_method_id ORDER BY contractor_version_to_contact_method.array_order_index asc separator ',')
FROM contractor_version_to_contact_method WHERE contractor_version_to_contact_method.contractor_version_id = v.id
) as proposed_suggestion_change_ids,
s.created_at,
v.effective_at,
v.created_at as updated_at
FROM contractor s
JOIN contractor_cvp cvp ON s.id = cvp.contractor_id
JOIN contractor_version v ON v.id = cvp.contractor_version_id;
;
`;

// apply the tables needed to apply the view
await connection.query({ sql: 'DROP TABLE IF EXISTS contractor' });
await connection.query({
sql: 'CREATE TABLE contractor ( id BIGINT, uuid VARCHAR(255), name VARCHAR(255), created_at DATETIME )',
});
await connection.query({ sql: 'DROP TABLE IF EXISTS contractor_version' });
await connection.query({
sql:
'CREATE TABLE contractor_version ( id BIGINT, contractor_id BIGINT, created_at DATETIME, effective_at DATETIME)',
});

await connection.query({ sql: 'DROP TABLE IF EXISTS contractor_cvp' });
await connection.query({
sql: 'CREATE TABLE contractor_cvp ( contractor_id BIGINT, contractor_version_id BIGINT)',
});
await connection.query({ sql: 'DROP TABLE IF EXISTS contractor_version_to_contractor_license' });
await connection.query({
sql:
'CREATE TABLE contractor_version_to_contractor_license ( contractor_version_id BIGINT, contractor_license_id BIGINT, array_order_index INT )',
});
await connection.query({ sql: 'DROP TABLE IF EXISTS contractor_version_to_contact_method' });
await connection.query({
sql:
'CREATE TABLE contractor_version_to_contact_method ( contractor_version_id BIGINT, contact_method_id BIGINT, array_order_index INT )',
});

// apply the sql
await connection.query({ sql: 'DROP VIEW IF EXISTS view_contractor_current;' }); // ensure possible previous state does not affect test
await connection.query({ sql: userDefSql });

// get get the SHOW CREATE sql
const liveResource = await getLiveResourceDefinitionFromDatabase({
connection,
resourceName: 'view_contractor_current',
resourceType: ResourceType.VIEW,
});
const showCreateDefSql = liveResource.sql;

// check that we normalize to the same thing
const normalizedUserSqlDef = normalizeDDLToSupportLossyShowCreateStatements({
ddl: userDefSql,
resourceType: ResourceType.VIEW,
});
const normalizedShowCreateDefSql = stripIrrelevantContentFromResourceDDL({
ddl: normalizeDDLToSupportLossyShowCreateStatements({
ddl: showCreateDefSql,
resourceType: ResourceType.VIEW,
}),
resourceType: ResourceType.VIEW,
});
expect(normalizedShowCreateDefSql).toEqual(normalizedUserSqlDef);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,12 @@ where 1=1
});
expect(cleanedUserDef).toEqual(cleanedShowCreateDef);
});
it('should be able to normalize this ddl so that show create def looks reasonable', () => {
const showCreateDdlExample =
"CREATE ALGORITHM=UNDEFINED DEFINER=`root`@`%` SQL SECURITY DEFINER VIEW `view_contractor_current` AS select `s`.`id` AS `id`,`s`.`uuid` AS `uuid`,`s`.`name` AS `name`,(select group_concat(`contractor_version_to_contractor_license`.`contractor_license_id` order by `contractor_version_to_contractor_license`.`array_order_index` ASC separator ',') from `contractor_version_to_contractor_license` where (`contractor_version_to_contractor_license`.`contractor_version_id` = `v`.`id`)) AS `license_ids`,(select group_concat(`contractor_version_to_contact_method`.`contact_method_id` order by `contractor_version_to_contact_method`.`array_order_index` ASC separator ',') from `contractor_version_to_contact_method` where (`contractor_version_to_contact_method`.`contractor_version_id` = `v`.`id`)) AS `proposed_suggestion_change_ids`,`s`.`created_at` AS `created_at`,`v`.`effective_at` AS `effective_at`,`v`.`created_at` AS `updated_at` from ((`contractor` `s` join `contractor_cvp` `cvp` on((`s`.`id` = `cvp`.`contractor_id`))) join `contractor_version` `v` on((`v`.`id` = `cvp`.`contractor_version_id`)))";
normalizeDDLToSupportLossyShowCreateStatements({
ddl: showCreateDdlExample,
resourceType: ResourceType.VIEW,
});
});
});
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { ResourceType } from '../../../../../types';
import strip from 'sql-strip-comments';
import sqlFormatter from 'sql-formatter';
import { heavilyNormalizeViewDDL } from './heavilyNormalizeViewDDL';
import strip from 'sql-strip-comments';

import { ResourceType } from '../../../../../types';
import { recursivelyHeavilyNormalizeViewDdl } from './recursivelyHeavilyNormalizeViewDdl/recursivelyHeavilyNormalizeViewDdl';

const RESOURCES_WITH_LOSSY_SHOW_CREATE_STATEMENTS = [ResourceType.TABLE, ResourceType.VIEW];

Expand All @@ -28,7 +29,7 @@ export const normalizeDDLToSupportLossyShowCreateStatements = ({
// if view, fix a few fun "features" that the SHOW CREATE statement adds
const extraNormalizedDDL =
resourceType === ResourceType.VIEW
? heavilyNormalizeViewDDL({ ddl: strippedDDL }) // SHOW CREATE for view has a lot of fun features, so extra normalization is needed
? recursivelyHeavilyNormalizeViewDdl({ ddl: strippedDDL }) // SHOW CREATE for view has a lot of fun features, so extra normalization is needed
: strippedDDL;

// apply formatting, since views have bad formatting
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`removeParenthesisSurroundingJoinsInFromClause should remove them from every join 1`] = `
"select
s.id as id,
s.uuid as uuid,
s.name as name,
s.created_at as created_at,
v.effective_at as effective_at,
v.created_at as updated_at
from
contractor s
join contractor_cvp cvp on s.id = cvp.contractor_id
join contractor_version v on v.id = cvp.contractor_version_id
"
`;
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`removeRedundantAliasDeclarations redundant declarations to be removed and nonredundant to remain 1`] = `
"
CREATE VIEW \`view_contractor_current\` AS
SELECT
s.id,
s.uuid,
s.name,
(
SELECT GROUP_CONCAT(contractor_version_to_contractor_license.contractor_license_id ORDER BY contractor_version_to_contractor_license.array_order_index)
FROM contractor_version_to_contractor_license WHERE contractor_version_to_contractor_license.contractor_version_id = v.id
) as license_ids,
(
SELECT GROUP_CONCAT(contractor_version_to_contact_method.contact_method_id ORDER BY contractor_version_to_contact_method.array_order_index)
FROM contractor_version_to_contact_method WHERE contractor_version_to_contact_method.contractor_version_id = v.id
) as proposed_suggestion_change_ids,
s.created_at,
v.effective_at,
v.created_at as updated_at
FROM contractor s
JOIN contractor_cvp cvp ON s.id = cvp.contractor_id
JOIN contractor_version v ON v.id = cvp.contractor_version_id;
"
`;
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
/**
* replace `,(` patterns w/ space in between, since our formatter downstream does not like that
*/
export const addSpacesBetweenCommaParenthesisOccurances = ({ ddl }: { ddl: string }) => {
return ddl.replace(/,\(/g, ', (');
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/**
* make all "as" words, except the first, lowercase
*
* for some reason, SHOW CREATE lowercases all tokens EXCEPT `as`, which it explicitly upper cases...)
*/
export const lowercaseAllAsTokensExceptFirstOne = ({ ddl }: { ddl: string }) => {
return ddl
.replace(/ AS /g, ' as ') // /g to apply to each
.replace(/ as /, ' AS '); // no /g so only apply to first case
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/**
* lowercase everything in the query, since show create does :upside_down_smiley:
*/
export const lowercaseEverythingAfterFirstAs = ({ ddl }: { ddl: string }) => {
const casedAsDdlParts = ddl.split(' AS ');
const casedDdl = [casedAsDdlParts[0], casedAsDdlParts[1].toLowerCase()].join(' AS ');
return casedDdl;
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/**
* strip out all of the back ticks.
*
* SHOW CREATE puts backticks __everywhere__
*
* until we have an example where that breaks something, they're just too much to force users to have to maintain
*/
export const removeAllBackticks = ({ ddl }: { ddl: string }) => ddl.replace(/`/g, '');
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
/**
* strip out the "double parenthesis" that SHOW CREATE likes to put on the "join on" statements
*/
export const removeDoubleParenthesisInJoinOnConditions = ({ sql }: { sql: string }) => {
return sql.replace(/ on\(\((\w+\.\w+\s=\s\w+\.\w+)\)\)/g, ' on $1 '); // note: strictly only do this if matching the SHOW CREATE weird format of ` on((tableA.column = tableB.column))`
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
/**
* show create drops the final semicolon. it really doesnt matter if the user still has theirs
*/
export const removeFinalSemicolon = ({ ddl }: { ddl: string }) => {
return ddl.replace(/;/g, '');
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/**
* strip out all of the parenthesis (since SHOW CREATE adds them everywhere and thats too much to force users to maintain); unideal since it modifies the meaning, but this is only relevant for diffing
*/
export const removeParenthesisFromWhereConditions = ({ flattenedSql }: { flattenedSql: string }) => {
const whereCasing = !!flattenedSql.match(/\sWHERE\s/) ? 'WHERE' : 'where'; // determine if user is using upper case or lower case "where"
const sqlSplitOnWhere = flattenedSql.split(whereCasing);
if (sqlSplitOnWhere.length > 2) {
// should not occur because the sql should have been flattened already
throw new Error('found more than two parts of DDL after splitting on where; unexpected'); // fail fast
}
const sqlWithoutParens =
sqlSplitOnWhere.length === 2
? sqlSplitOnWhere[0] + '\n' + whereCasing + '\n' + sqlSplitOnWhere[1].replace(/[\(\)]/g, '') // tslint:disable-line prefer-template
: sqlSplitOnWhere[0]; // if no where clause, then nothing to replace
return sqlWithoutParens;
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { removeParenthesisSurroundingJoinsInFromClause } from './removeParenthesisSurroundingJoinsInFromClause';

describe('removeParenthesisSurroundingJoinsInFromClause', () => {
it('should remove them from every join', () => {
const exampleSql = `
select
s.id as id,
s.uuid as uuid,
s.name as name,
s.created_at as created_at,
v.effective_at as effective_at,
v.created_at as updated_at
from
(
(
contractor s
join contractor_cvp cvp on s.id = cvp.contractor_id
)
join contractor_version v on v.id = cvp.contractor_version_id
)
`.trim();
const normalizedSql = removeParenthesisSurroundingJoinsInFromClause({ flattenedSql: exampleSql });
expect(normalizedSql).not.toContain('(');
expect(normalizedSql).not.toContain(')');
expect(normalizedSql).toMatchSnapshot();
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/**
* strip out the first and last paren of the "FROM" section, IF it is defined;
*
* show create likes to wrap the whole "FROM" section w/ parens... its unreasonable to ask people to do that too.
*/
export const removeParenthesisSurroundingJoinsInFromClause = ({ flattenedSql }: { flattenedSql: string }) => {
// check that this sql has this situation going on, if not, do nothing
const ddlHasParenOpenRightAfterFromClause = !!flattenedSql.match(/\sfrom\s+\(/gi); // note, we're not worried about subqueries because we expect flattenedSql as input
if (!ddlHasParenOpenRightAfterFromClause) return flattenedSql; // do nothing if the situation does not exist

// split DDL on the from statement, since we know it exists
const flattenedSqlParts = flattenedSql.split(/\sfrom\s/gi);
if (flattenedSqlParts.length !== 2) throw new Error('not exactly two parts after splitting on from; unexpected'); // fail fast

// remove the parens that encapsulate each join
const fromClauseWithoutParens = flattenedSqlParts[1].replace(/\(/g, '').replace(/\)/g, ''); // note: we replace all parens, since subqueries are taken care of

// join them back and the ddl w/o this mess
const flattenedSqlWithoutOpenCloseParenInFromClause = flattenedSqlParts[0] + '\nfrom\n' + fromClauseWithoutParens; // tslint:disable-line prefer-template

// return without the parens
return flattenedSqlWithoutOpenCloseParenInFromClause;
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import { removeRedundantAliasDeclarations } from './removeRedundantAliasDeclarations';

describe('removeRedundantAliasDeclarations', () => {
test('redundant declarations to be removed and nonredundant to remain', () => {
const exampleSql = `
CREATE VIEW \`view_contractor_current\` AS
SELECT
s.id,
s.uuid as uuid,
s.name as name,
(
SELECT GROUP_CONCAT(contractor_version_to_contractor_license.contractor_license_id ORDER BY contractor_version_to_contractor_license.array_order_index)
FROM contractor_version_to_contractor_license WHERE contractor_version_to_contractor_license.contractor_version_id = v.id
) as license_ids,
(
SELECT GROUP_CONCAT(contractor_version_to_contact_method.contact_method_id ORDER BY contractor_version_to_contact_method.array_order_index)
FROM contractor_version_to_contact_method WHERE contractor_version_to_contact_method.contractor_version_id = v.id
) as proposed_suggestion_change_ids,
s.created_at,
v.effective_at,
v.created_at as updated_at
FROM contractor s
JOIN contractor_cvp cvp ON s.id = cvp.contractor_id
JOIN contractor_version v ON v.id = cvp.contractor_version_id;
`;
const normalizedSql = removeRedundantAliasDeclarations({ sql: exampleSql });
expect(normalizedSql).not.toContain('as uuid');
expect(normalizedSql).not.toContain('as name');
expect(normalizedSql).toMatchSnapshot(); // for visual inspection to make sure no other defects occured
});
});
Loading

0 comments on commit f6e9f7f

Please sign in to comment.