From 3069a689fe7dcca14200a6ed1ba1fdff3a31dbf8 Mon Sep 17 00:00:00 2001
From: Eric Banks <ebanks@broadinstitute.org>
Date: Mon, 19 Dec 2011 10:04:33 -0500
Subject: [PATCH] Bug fix: if there  are multiple records at a given position,
 it turns out that SelectVariants would drop all variants that follow after
 one that fails filters (instead of dropping just the failing one).  Added an
 integration test to cover this case.

---
 .../walkers/variantutils/SelectVariants.java  | 20 +++++++++++--------
 .../SelectVariantsIntegrationTest.java        | 13 ++++++++++++
 2 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/public/java/src/org/broadinstitute/sting/gatk/walkers/variantutils/SelectVariants.java b/public/java/src/org/broadinstitute/sting/gatk/walkers/variantutils/SelectVariants.java
index 0af351750..6d94ffe6d 100755
--- a/public/java/src/org/broadinstitute/sting/gatk/walkers/variantutils/SelectVariants.java
+++ b/public/java/src/org/broadinstitute/sting/gatk/walkers/variantutils/SelectVariants.java
@@ -493,12 +493,12 @@ public class SelectVariants extends RodWalker<Integer, Integer> implements TreeR
             if (DISCORDANCE_ONLY) {
                 Collection<VariantContext> compVCs = tracker.getValues(discordanceTrack, context.getLocation());
                 if (!isDiscordant(vc, compVCs))
-                    return 0;
+                    continue;
             }
             if (CONCORDANCE_ONLY) {
                 Collection<VariantContext> compVCs = tracker.getValues(concordanceTrack, context.getLocation());
                 if (!isConcordant(vc, compVCs))
-                    return 0;
+                    continue;
             }
 
             if (alleleRestriction.equals(NumberAlleleRestriction.BIALLELIC) && !vc.isBiallelic())
@@ -512,16 +512,20 @@ public class SelectVariants extends RodWalker<Integer, Integer> implements TreeR
 
             VariantContext sub = subsetRecord(vc, samples);
             if ( (sub.isPolymorphicInSamples() || !EXCLUDE_NON_VARIANTS) && (!sub.isFiltered() || !EXCLUDE_FILTERED) ) {
+                boolean failedJexlMatch = false;
                 for ( VariantContextUtils.JexlVCMatchExp jexl : jexls ) {
                     if ( !VariantContextUtils.match(sub, jexl) ) {
-                        return 0;
+                        failedJexlMatch = true;
+                        break;
                     }
                 }
-                if (SELECT_RANDOM_NUMBER) {
-                    randomlyAddVariant(++variantNumber, sub);
-                }
-                else if (!SELECT_RANDOM_FRACTION || ( GenomeAnalysisEngine.getRandomGenerator().nextDouble() < fractionRandom)) {
-                    vcfWriter.add(sub);
+                if ( !failedJexlMatch ) {
+                    if (SELECT_RANDOM_NUMBER) {
+                        randomlyAddVariant(++variantNumber, sub);
+                    }
+                    else if (!SELECT_RANDOM_FRACTION || ( GenomeAnalysisEngine.getRandomGenerator().nextDouble() < fractionRandom)) {
+                        vcfWriter.add(sub);
+                    }
                 }
             }
         }
diff --git a/public/java/test/org/broadinstitute/sting/gatk/walkers/variantutils/SelectVariantsIntegrationTest.java b/public/java/test/org/broadinstitute/sting/gatk/walkers/variantutils/SelectVariantsIntegrationTest.java
index 72a07bd0e..042de2a27 100755
--- a/public/java/test/org/broadinstitute/sting/gatk/walkers/variantutils/SelectVariantsIntegrationTest.java
+++ b/public/java/test/org/broadinstitute/sting/gatk/walkers/variantutils/SelectVariantsIntegrationTest.java
@@ -116,6 +116,19 @@ public class SelectVariantsIntegrationTest extends WalkerTest {
         executeTest("testUsingDbsnpName--" + testFile, spec);
     }
 
+    @Test
+    public void testMultipleRecordsAtOnePosition() {
+        String testFile = validationDataLocation + "selectVariants.onePosition.vcf";
+
+        WalkerTestSpec spec = new WalkerTestSpec(
+                "-T SelectVariants -R " + b36KGReference + " -select 'KG_FREQ < 0.5' --variant " + testFile + " -o %s -NO_HEADER",
+                1,
+                Arrays.asList("20b52c96f5c48258494d072752b53693")
+        );
+
+        executeTest("testMultipleRecordsAtOnePositionFirstIsFiltered--" + testFile, spec);
+    }
+
     @Test
     public void testParallelization() {
         String testfile = validationDataLocation + "test.filtered.maf_annotated.vcf";