Fix WHERE clause for non-assessment Teradata tasks (#341)

The WHERE clause generated for Teradata logs tasks (in `teradata-logs` connector without specifying the `--assessment` flag) contained repeated conditions `L.UserName <> 'DBC'`: ``` L.UserName <> 'DBC' AND L.UserName <> 'DBC' AND L.UserName <> 'DBC' AND ... ``` The `conditions` list was mistakenly modified inside the loop, which caused additional redundant conditions to be added to the query, if there was more than one time interval extracted. If there were too many such conditions, the query failed.
google · Jan 22, 2024 · 9162ae5 · 9162ae5
1 parent a5cdefc
commit 9162ae5
Show file tree

Hide file tree

Showing 7 changed files with 278 additions and 26 deletions.
diff --git a/...igration/dumper/application/dumper/connector/teradata/TeradataAssessmentLogsJdbcTask.java b/...igration/dumper/application/dumper/connector/teradata/TeradataAssessmentLogsJdbcTask.java
@@ -22,6 +22,7 @@
 import com.google.edwmigration.dumper.application.dumper.connector.teradata.AbstractTeradataConnector.SharedState;
 import java.util.List;
 import java.util.OptionalLong;
+import java.util.Set;
 import java.util.function.Predicate;
 import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
@@ -160,7 +161,7 @@ public TeradataAssessmentLogsJdbcTask(
       SharedState state,
       String logTable,
       String queryTable,
-      List<String> conditions,
+      Set<String> conditions,
       ZonedInterval interval,
       @CheckForNull String logDateColumn,
       OptionalLong maxSqlLength,

diff --git a/...ogle/edwmigration/dumper/application/dumper/connector/teradata/TeradataLogsConnector.java b/...ogle/edwmigration/dumper/application/dumper/connector/teradata/TeradataLogsConnector.java
@@ -21,6 +21,7 @@
 import com.google.auto.service.AutoService;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Range;
 import com.google.edwmigration.dumper.application.dumper.ConnectorArguments;
 import com.google.edwmigration.dumper.application.dumper.MetadataDumperUsageException;
@@ -44,7 +45,6 @@
 import com.google.edwmigration.dumper.plugin.lib.dumper.spi.TeradataLogsDumpFormat;
 import java.time.Duration;
 import java.time.format.DateTimeFormatter;
-import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 import java.util.OptionalLong;
@@ -189,13 +189,13 @@ public void addTasksTo(List<? super Task<?>> out, @Nonnull ConnectorArguments ar
       logTable = alternates.get(0);
       queryTable = alternates.get(1);
     }
-    List<String> conditions = new ArrayList<>();
+    ImmutableSet.Builder<String> conditionsBuilder = ImmutableSet.builder();
     // if the user specifies an earliest start time there will be extraneous empty dump files
     // because we always iterate over the full 7 trailing days; maybe it's worth
     // preventing that in the future. To do that, we should require getQueryLogEarliestTimestamp()
     // to parse and return an ISO instant, not a database-server-specific format.
     if (!StringUtils.isBlank(arguments.getQueryLogEarliestTimestamp())) {
-      conditions.add("L.StartTime >= " + arguments.getQueryLogEarliestTimestamp());
+      conditionsBuilder.add("L.StartTime >= " + arguments.getQueryLogEarliestTimestamp());
     }
 
     Duration rotationDuration = arguments.getQueryLogRotationFrequency();
@@ -211,6 +211,10 @@ public void addTasksTo(List<? super Task<?>> out, @Nonnull ConnectorArguments ar
     OptionalLong maxSqlLength =
         PropertyParser.parseNumber(
             arguments, TeradataLogsConnectorProperty.MAX_SQL_LENGTH, MAX_SQL_LENGTH_RANGE);
+    if (!isAssessment) {
+      conditionsBuilder.add("L.UserName <> 'DBC'");
+    }
+    ImmutableSet<String> conditions = conditionsBuilder.build();
     for (ZonedInterval interval : intervals) {
       String file = createFilename(ZIP_ENTRY_PREFIX, interval);
       if (isAssessment) {
@@ -235,7 +239,6 @@ public void addTasksTo(List<? super Task<?>> out, @Nonnull ConnectorArguments ar
                 utilityLogsTable,
                 interval));
       } else {
-        conditions.add("L.UserName <> 'DBC'");
         out.add(
             new TeradataLogsJdbcTask(
                     file, queryLogsState, logTable, queryTable, conditions, interval)

diff --git a/...oogle/edwmigration/dumper/application/dumper/connector/teradata/TeradataLogsJdbcTask.java b/...oogle/edwmigration/dumper/application/dumper/connector/teradata/TeradataLogsJdbcTask.java
@@ -29,6 +29,7 @@
 import com.google.common.base.Preconditions;
 import com.google.common.base.Predicates;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.io.ByteSink;
 import com.google.common.primitives.Ints;
 import com.google.edwmigration.dumper.application.dumper.connector.ZonedInterval;
@@ -43,9 +44,9 @@
 import java.sql.SQLException;
 import java.time.ZoneOffset;
 import java.time.format.DateTimeFormatter;
-import java.util.Collections;
 import java.util.List;
 import java.util.OptionalLong;
+import java.util.Set;
 import java.util.function.Function;
 import java.util.function.Predicate;
 import javax.annotation.CheckForNull;
@@ -100,18 +101,18 @@ public class TeradataLogsJdbcTask extends AbstractJdbcTask<Summary> {
   protected final SharedState state;
   protected final String logTable;
   protected final String queryTable;
-  protected final List<String> conditions;
+  protected final ImmutableSet<String> conditions;
   protected final ZonedInterval interval;
   @CheckForNull private final String logDateColumn;
   private final OptionalLong maxSqlLength;
-  protected final List<String> orderBy;
+  protected final ImmutableList<String> orderBy;
 
   public TeradataLogsJdbcTask(
       @Nonnull String targetPath,
       SharedState state,
       String logTable,
       String queryTable,
-      List<String> conditions,
+      Set<String> conditions,
       ZonedInterval interval) {
     this(
         targetPath,
@@ -122,15 +123,15 @@ public TeradataLogsJdbcTask(
         interval,
         /* logDateColumn= */ null,
         /* maxSqlLength= */ OptionalLong.empty(),
-        Collections.emptyList());
+        /* orderBy= */ ImmutableList.of());
   }
 
   protected TeradataLogsJdbcTask(
       @Nonnull String targetPath,
       SharedState state,
       String logTable,
       String queryTable,
-      List<String> conditions,
+      Set<String> conditions,
       ZonedInterval interval,
       @CheckForNull String logDateColumn,
       OptionalLong maxSqlLength,
@@ -139,11 +140,11 @@ protected TeradataLogsJdbcTask(
     this.state = Preconditions.checkNotNull(state, "SharedState was null.");
     this.logTable = logTable;
     this.queryTable = queryTable;
-    this.conditions = conditions;
+    this.conditions = ImmutableSet.copyOf(conditions);
     this.interval = interval;
     this.logDateColumn = logDateColumn;
     this.maxSqlLength = maxSqlLength;
-    this.orderBy = orderBy;
+    this.orderBy = ImmutableList.copyOf(orderBy);
   }
 
   private static boolean isQueryTable(@Nonnull String expression) {
@@ -248,7 +249,7 @@ private String getSql(@Nonnull JdbcHandle handle) {
       buf.append(" ORDER BY ");
       Joiner.on(", ").appendTo(buf, orderBy);
     }
-    return buf.toString().replace('\n', ' ');
+    return formatQuery(buf.toString());
   }
 
   private String createLogDateColumnConditionStr() {

diff --git a/...va/com/google/edwmigration/dumper/application/dumper/connector/AbstractConnectorTest.java b/...va/com/google/edwmigration/dumper/application/dumper/connector/AbstractConnectorTest.java
@@ -40,7 +40,7 @@ public abstract class AbstractConnectorTest {
   @SuppressWarnings("UnusedVariable")
   private static final Logger LOG = LoggerFactory.getLogger(AbstractConnectorTest.class);
 
-  protected static enum SpecialTaskType {
+  protected enum SpecialTaskType {
     Version,
     Arguments,
     Format

diff --git a/...tion/dumper/application/dumper/connector/teradata/TeradataAssessmentLogsJdbcTaskTest.java b/...tion/dumper/application/dumper/connector/teradata/TeradataAssessmentLogsJdbcTaskTest.java
@@ -20,6 +20,7 @@
 import static java.util.Collections.emptyList;
 
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
 import com.google.edwmigration.dumper.application.dumper.connector.ZonedInterval;
 import com.google.edwmigration.dumper.application.dumper.connector.teradata.AbstractTeradataConnector.SharedState;
 import java.time.ZoneId;
@@ -52,7 +53,7 @@ public void getSql_success() {
             queryLogsState,
             "SampleQueryTable",
             "SampleSqlTable",
-            /* conditions= */ emptyList(),
+            /* conditions= */ ImmutableSet.of(),
             interval,
             /* logDateColumn= */ null,
             /* maxSqlLength= */ OptionalLong.empty(),
@@ -79,7 +80,7 @@ public void getSql_maxSqlLength() {
             queryLogsState,
             "SampleQueryTable",
             "SampleSqlTable",
-            /* conditions= */ emptyList(),
+            /* conditions= */ ImmutableSet.of(),
             interval,
             /* logDateColumn= */ null,
             /* maxSqlLength= */ OptionalLong.of(20000),
@@ -114,7 +115,7 @@ public void getSql_noSecondTable() {
             queryLogsState,
             "SampleQueryTable",
             "SampleSqlTable",
-            /* conditions= */ emptyList(),
+            /* conditions= */ ImmutableSet.of(),
             interval,
             /* logDateColumn= */ null,
             /* maxSqlLength= */ OptionalLong.empty(),
@@ -141,7 +142,7 @@ public void getSql_noSecondTableWithLogDateColumn() {
             queryLogsState,
             "SampleQueryTable",
             "SampleSqlTable",
-            /* conditions= */ emptyList(),
+            /* conditions= */ ImmutableSet.of(),
             interval,
             "SampleLogDate", /* orderBy */
             /* maxSqlLength= */ OptionalLong.empty(),
@@ -169,7 +170,7 @@ public void getSql_noSecondTableWithCondition() {
             queryLogsState,
             "SampleQueryTable",
             "SampleSqlTable",
-            /* conditions= */ ImmutableList.of("L.QueryID=7"),
+            /* conditions= */ ImmutableSet.of("L.QueryID=7"),
             interval,
             /* logDateColumn= */ null,
             /* maxSqlLength= */ OptionalLong.empty(),
@@ -196,7 +197,7 @@ public void getSql_noSecondTableWithOrderBy() {
             queryLogsState,
             "SampleQueryTable",
             "SampleSqlTable",
-            /* conditions= */ emptyList(),
+            /* conditions= */ ImmutableSet.of(),
             interval,
             /* logDateColumn= */ null,
             /* maxSqlLength= */ OptionalLong.empty(),
@@ -224,7 +225,7 @@ public void getSql_withLogDateColumn() {
             queryLogsState,
             "SampleQueryTable",
             "SampleSqlTable",
-            /* conditions= */ emptyList(),
+            /* conditions= */ ImmutableSet.of(),
             interval,
             "SampleLogDate", /* orderBy */
             /* maxSqlLength= */ OptionalLong.empty(),
@@ -253,7 +254,7 @@ public void getSql_withLogDateColumnAndMaxSqlLength() {
             queryLogsState,
             "SampleQueryTable",
             "SampleSqlTable",
-            /* conditions= */ emptyList(),
+            /* conditions= */ ImmutableSet.of(),
             interval,
             "SampleLogDate", /* orderBy */
             /* maxSqlLength= */ OptionalLong.of(20000),
@@ -292,7 +293,7 @@ public void getSql_fullQuery() {
             queryLogsState,
             "SampleQueryTable",
             "SampleSqlTable",
-            ImmutableList.of("QueryID=7", "QueryText LIKE '%abc%'"),
+            ImmutableSet.of("QueryID=7", "QueryText LIKE '%abc%'"),
             interval,
             "SampleLogDate",
             /* maxSqlLength= */ OptionalLong.empty(),