From 20f4e44e69ef6f9444a08e7b78c0871bbc688ef9 Mon Sep 17 00:00:00 2001 From: Patrick Duin Date: Thu, 4 Mar 2021 18:50:48 +0100 Subject: [PATCH] skip presto view parsing (#211) --- 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..44710eae6 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 views 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..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 @@ -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 (hasNonHiveViewMarker(query)) { + // skipping queries that are not "Hive" view queries. We can't parse those. + return query; + } ASTNode root; try { root = ParseUtils.parse(query); @@ -56,6 +61,13 @@ public String transformOutboundDatabaseName(MetaStoreMapping metaStoreMapping, S return result.toString(); } + private boolean hasNonHiveViewMarker(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..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 @@ -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 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; + } 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..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 @@ -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(query)); + } + + @Test + public void transformOutboundDatabaseNamePrestoExpandedTextMarker() { + ASTQueryMapping queryMapping = ASTQueryMapping.INSTANCE; + + String query = "/* Presto View: */"; + + assertThat(queryMapping.transformOutboundDatabaseName(metaStoreMapping, query), + is(query)); } @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";