From 0deda8d8d65e6808677d0ceab14c6933c321734a Mon Sep 17 00:00:00 2001 From: patduin Date: Thu, 4 Mar 2021 15:23:30 +0100 Subject: [PATCH 1/7] skip presto view parsing --- CHANGELOG.md | 1 + .../mapping/model/ASTQueryMapping.java | 12 ++++++++++ .../mapping/model/DatabaseMappingImpl.java | 10 +++++++- .../mapping/model/ASTQueryMappingTest.java | 23 +++++++++++++++++-- .../model/DatabaseMappingImplTest.java | 14 +++++++++++ 5 files changed, 57 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 98e617afb..7885a45b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ ## [3.9.1] - TBD ### Fixed * Null pointer exception when creating a metastore tunnel by adding a check for null `configuration-properties`. +* Fixing issue where Presto view are cannot be parsed resulting in errors. ## [3.9.0] - 2021-02-26 ### Added diff --git a/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/mapping/model/ASTQueryMapping.java b/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/mapping/model/ASTQueryMapping.java index ff1ea08fd..4ddecd823 100644 --- a/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/mapping/model/ASTQueryMapping.java +++ b/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/mapping/model/ASTQueryMapping.java @@ -39,11 +39,16 @@ public enum ASTQueryMapping implements QueryMapping { INSTANCE; + private static final String PRESTO_VIEW_MARKER = "/* Presto View"; private final static String RE_WORD_BOUNDARY = "\\b"; private final static Comparator ON_START_INDEX = Comparator.comparingInt(CommonToken::getStartIndex); @Override public String transformOutboundDatabaseName(MetaStoreMapping metaStoreMapping, String query) { + if (hasMarker(query)) { + // skipping view that are not "Hive" views queries. We can't parse those at this moment. + return query; + } ASTNode root; try { root = ParseUtils.parse(query); @@ -56,6 +61,13 @@ public String transformOutboundDatabaseName(MetaStoreMapping metaStoreMapping, S return result.toString(); } + boolean hasMarker(String query) { + if (query != null && query.trim().startsWith(PRESTO_VIEW_MARKER)) { + return true; + } + return false; + } + private StringBuilder transformDatabaseTableTokens(MetaStoreMapping metaStoreMapping, ASTNode root, String query) { StringBuilder result = new StringBuilder(); SortedSet dbNameTokens = extractDbNameTokens(root); diff --git a/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/mapping/model/DatabaseMappingImpl.java b/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/mapping/model/DatabaseMappingImpl.java index 48b92a585..2dd23c368 100644 --- a/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/mapping/model/DatabaseMappingImpl.java +++ b/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/mapping/model/DatabaseMappingImpl.java @@ -88,7 +88,15 @@ public MetaStoreFilterHook getMetastoreFilter() { @Override public Table transformOutboundTable(Table table) { - table.setDbName(metaStoreMapping.transformOutboundDatabaseName(table.getDbName())); + String originalDatabaseName = table.getDbName(); + String databaseName = metaStoreMapping.transformOutboundDatabaseName(originalDatabaseName); + table.setDbName(databaseName); + if (databaseName.equalsIgnoreCase(originalDatabaseName)) { + // Skip all the view stuff if nothing is going to change, the parsing is not without problems and we can't catch + // all use cases here. For instance Presto creates views that are stored in these fields this is stored + // differently than Hive. There might be others. + return table; + } if (table.isSetViewExpandedText()) { try { log.debug("Transforming ViewExpandedText: {}", table.getViewExpandedText()); diff --git a/waggle-dance-core/src/test/java/com/hotels/bdp/waggledance/mapping/model/ASTQueryMappingTest.java b/waggle-dance-core/src/test/java/com/hotels/bdp/waggledance/mapping/model/ASTQueryMappingTest.java index eb3e70c5e..5c9f8aef9 100644 --- a/waggle-dance-core/src/test/java/com/hotels/bdp/waggledance/mapping/model/ASTQueryMappingTest.java +++ b/waggle-dance-core/src/test/java/com/hotels/bdp/waggledance/mapping/model/ASTQueryMappingTest.java @@ -38,8 +38,27 @@ public class ASTQueryMappingTest { @Before public void setUp() { - metaStoreMapping = new PrefixMapping(new MetaStoreMappingImpl(PREFIX, "mapping", null, - null, DIRECT, LATENCY, new DefaultMetaStoreFilterHookImpl(new HiveConf()))); + metaStoreMapping = new PrefixMapping(new MetaStoreMappingImpl(PREFIX, "mapping", null, null, DIRECT, LATENCY, + new DefaultMetaStoreFilterHookImpl(new HiveConf()))); + } + + @Test + public void transformOutboundDatabaseNamePrestoMarker() { + ASTQueryMapping queryMapping = ASTQueryMapping.INSTANCE; + + String query = "/* Presto View */"; + + assertThat(queryMapping.transformOutboundDatabaseName(metaStoreMapping, query), is("/* Presto View */")); + } + + @Test + public void transformOutboundDatabaseNamePrestoExpandedTextMarker() { + ASTQueryMapping queryMapping = ASTQueryMapping.INSTANCE; + + String query = "/* Presto View: */"; + + assertThat(queryMapping.transformOutboundDatabaseName(metaStoreMapping, query), + is("/* Presto View: */")); } @Test diff --git a/waggle-dance-core/src/test/java/com/hotels/bdp/waggledance/mapping/model/DatabaseMappingImplTest.java b/waggle-dance-core/src/test/java/com/hotels/bdp/waggledance/mapping/model/DatabaseMappingImplTest.java index ddd53cc93..462ee7e73 100644 --- a/waggle-dance-core/src/test/java/com/hotels/bdp/waggledance/mapping/model/DatabaseMappingImplTest.java +++ b/waggle-dance-core/src/test/java/com/hotels/bdp/waggledance/mapping/model/DatabaseMappingImplTest.java @@ -159,6 +159,20 @@ public void transformOutboundTableView() throws Exception { assertThat(result.getViewOriginalText(), is(VIEW_ORIGINAL_TEXT_TRANSFORMED)); } + @Test + public void transformOutboundTableViewIgnoreParsingWhenSame() throws Exception { + Table table = new Table(); + table.setDbName(OUT_DB_NAME); + table.setViewExpandedText(VIEW_EXPANDED_TEXT); + table.setViewOriginalText(VIEW_ORIGINAL_TEXT); + + Table result = databaseMapping.transformOutboundTable(table); + assertThat(result, is(sameInstance(table))); + assertThat(result.getDbName(), is(OUT_DB_NAME)); + assertThat(result.getViewExpandedText(), is(VIEW_EXPANDED_TEXT)); + assertThat(result.getViewOriginalText(), is(VIEW_ORIGINAL_TEXT)); + } + @Test public void transformOutboundTableViewExpandedTextErrorKeepOriginal() throws Exception { String viewExpandedText = "error"; From 8cec0a97a18d1f7610ad9b8395a8a65c8d3830e1 Mon Sep 17 00:00:00 2001 From: patduin Date: Thu, 4 Mar 2021 15:25:27 +0100 Subject: [PATCH 2/7] typos --- .../hotels/bdp/waggledance/mapping/model/ASTQueryMapping.java | 2 +- .../bdp/waggledance/mapping/model/DatabaseMappingImpl.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/mapping/model/ASTQueryMapping.java b/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/mapping/model/ASTQueryMapping.java index 4ddecd823..c259b5a18 100644 --- a/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/mapping/model/ASTQueryMapping.java +++ b/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/mapping/model/ASTQueryMapping.java @@ -46,7 +46,7 @@ public enum ASTQueryMapping implements QueryMapping { @Override public String transformOutboundDatabaseName(MetaStoreMapping metaStoreMapping, String query) { if (hasMarker(query)) { - // skipping view that are not "Hive" views queries. We can't parse those at this moment. + // skipping query that are not "Hive" view queries. We can't parse those. return query; } ASTNode root; diff --git a/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/mapping/model/DatabaseMappingImpl.java b/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/mapping/model/DatabaseMappingImpl.java index 2dd23c368..eb6df5c3a 100644 --- a/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/mapping/model/DatabaseMappingImpl.java +++ b/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/mapping/model/DatabaseMappingImpl.java @@ -92,8 +92,8 @@ public Table transformOutboundTable(Table table) { String databaseName = metaStoreMapping.transformOutboundDatabaseName(originalDatabaseName); table.setDbName(databaseName); if (databaseName.equalsIgnoreCase(originalDatabaseName)) { - // Skip all the view stuff if nothing is going to change, the parsing is not without problems and we can't catch - // all use cases here. For instance Presto creates views that are stored in these fields this is stored + // Skip all the view parsing if nothing is going to change, the parsing is not without problems and we can't catch + // all use cases here. For instance Presto creates views that are stored in these fields and this is stored // differently than Hive. There might be others. return table; } From a0574be120f65f0164ac30299607e195527221b9 Mon Sep 17 00:00:00 2001 From: Patrick Duin Date: Thu, 4 Mar 2021 15:53:56 +0100 Subject: [PATCH 3/7] Update waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/mapping/model/ASTQueryMapping.java Co-authored-by: Andreea Paduraru <40387422+andreeapad@users.noreply.github.com> --- .../hotels/bdp/waggledance/mapping/model/ASTQueryMapping.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/mapping/model/ASTQueryMapping.java b/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/mapping/model/ASTQueryMapping.java index c259b5a18..dd81f7620 100644 --- a/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/mapping/model/ASTQueryMapping.java +++ b/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/mapping/model/ASTQueryMapping.java @@ -46,7 +46,7 @@ public enum ASTQueryMapping implements QueryMapping { @Override public String transformOutboundDatabaseName(MetaStoreMapping metaStoreMapping, String query) { if (hasMarker(query)) { - // skipping query that are not "Hive" view queries. We can't parse those. + // skipping queries that are not "Hive" view queries. We can't parse those. return query; } ASTNode root; From 8eb2a05ccb9f9a5ab5c6f362fe4f2b16d2f62cbf Mon Sep 17 00:00:00 2001 From: Patrick Duin Date: Thu, 4 Mar 2021 15:54:03 +0100 Subject: [PATCH 4/7] Update CHANGELOG.md Co-authored-by: Andreea Paduraru <40387422+andreeapad@users.noreply.github.com> --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7885a45b2..44710eae6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,7 @@ ## [3.9.1] - TBD ### Fixed * Null pointer exception when creating a metastore tunnel by adding a check for null `configuration-properties`. -* Fixing issue where Presto view are cannot be parsed resulting in errors. +* Fixing issue where Presto views cannot be parsed resulting in errors. ## [3.9.0] - 2021-02-26 ### Added From 59e346135ef28752d7e278a7d9d47b404b79fa97 Mon Sep 17 00:00:00 2001 From: Patrick Duin Date: Thu, 4 Mar 2021 15:54:47 +0100 Subject: [PATCH 5/7] Update waggle-dance-core/src/test/java/com/hotels/bdp/waggledance/mapping/model/ASTQueryMappingTest.java Co-authored-by: Adrian Woodhead --- .../bdp/waggledance/mapping/model/ASTQueryMappingTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/waggle-dance-core/src/test/java/com/hotels/bdp/waggledance/mapping/model/ASTQueryMappingTest.java b/waggle-dance-core/src/test/java/com/hotels/bdp/waggledance/mapping/model/ASTQueryMappingTest.java index 5c9f8aef9..2075a8bd5 100644 --- a/waggle-dance-core/src/test/java/com/hotels/bdp/waggledance/mapping/model/ASTQueryMappingTest.java +++ b/waggle-dance-core/src/test/java/com/hotels/bdp/waggledance/mapping/model/ASTQueryMappingTest.java @@ -48,7 +48,7 @@ public void transformOutboundDatabaseNamePrestoMarker() { String query = "/* Presto View */"; - assertThat(queryMapping.transformOutboundDatabaseName(metaStoreMapping, query), is("/* Presto View */")); + assertThat(queryMapping.transformOutboundDatabaseName(metaStoreMapping, query), is(query)); } @Test From a765d924253cc3ceab43ca9cc1d0a562ea2fffdb Mon Sep 17 00:00:00 2001 From: Patrick Duin Date: Thu, 4 Mar 2021 15:54:53 +0100 Subject: [PATCH 6/7] Update waggle-dance-core/src/test/java/com/hotels/bdp/waggledance/mapping/model/ASTQueryMappingTest.java Co-authored-by: Adrian Woodhead --- .../bdp/waggledance/mapping/model/ASTQueryMappingTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/waggle-dance-core/src/test/java/com/hotels/bdp/waggledance/mapping/model/ASTQueryMappingTest.java b/waggle-dance-core/src/test/java/com/hotels/bdp/waggledance/mapping/model/ASTQueryMappingTest.java index 2075a8bd5..aa94fd87a 100644 --- a/waggle-dance-core/src/test/java/com/hotels/bdp/waggledance/mapping/model/ASTQueryMappingTest.java +++ b/waggle-dance-core/src/test/java/com/hotels/bdp/waggledance/mapping/model/ASTQueryMappingTest.java @@ -58,7 +58,7 @@ public void transformOutboundDatabaseNamePrestoExpandedTextMarker() { String query = "/* Presto View: */"; assertThat(queryMapping.transformOutboundDatabaseName(metaStoreMapping, query), - is("/* Presto View: */")); + is(query)); } @Test From 35d47cf358e5398b99290611677629316a17b5bf Mon Sep 17 00:00:00 2001 From: patduin Date: Thu, 4 Mar 2021 17:34:17 +0100 Subject: [PATCH 7/7] rename --- .../hotels/bdp/waggledance/mapping/model/ASTQueryMapping.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/mapping/model/ASTQueryMapping.java b/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/mapping/model/ASTQueryMapping.java index dd81f7620..0bf8f8e05 100644 --- a/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/mapping/model/ASTQueryMapping.java +++ b/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/mapping/model/ASTQueryMapping.java @@ -45,7 +45,7 @@ public enum ASTQueryMapping implements QueryMapping { @Override public String transformOutboundDatabaseName(MetaStoreMapping metaStoreMapping, String query) { - if (hasMarker(query)) { + if (hasNonHiveViewMarker(query)) { // skipping queries that are not "Hive" view queries. We can't parse those. return query; } @@ -61,7 +61,7 @@ public String transformOutboundDatabaseName(MetaStoreMapping metaStoreMapping, S return result.toString(); } - boolean hasMarker(String query) { + private boolean hasNonHiveViewMarker(String query) { if (query != null && query.trim().startsWith(PRESTO_VIEW_MARKER)) { return true; }