From c29f1ebdd08d0479d6c360fe7f43908dda704fd6 Mon Sep 17 00:00:00 2001 From: marikomedlock Date: Fri, 13 Sep 2024 12:48:03 -0400 Subject: [PATCH] Boolean not filter missing parentheses. (#1003) --- .../query/sql/translator/ApiTranslator.java | 2 +- .../filter/BQBooleanLogicFilterTest.java | 33 +++++++++++++++++++ .../tanagra/query/sql/ApiTranslatorTest.java | 2 +- .../booleanNotFilter.sql | 2 +- ...eanNotFilterWithNestedBooleanAndFilter.sql | 8 +++++ .../booleanNotFilterWithSwapFields.sql | 4 +-- 6 files changed, 46 insertions(+), 5 deletions(-) create mode 100644 underlay/src/test/resources/sql/BQBooleanLogicFilterTest/booleanNotFilterWithNestedBooleanAndFilter.sql diff --git a/underlay/src/main/java/bio/terra/tanagra/query/sql/translator/ApiTranslator.java b/underlay/src/main/java/bio/terra/tanagra/query/sql/translator/ApiTranslator.java index 7a0503bd0..6c824e30d 100644 --- a/underlay/src/main/java/bio/terra/tanagra/query/sql/translator/ApiTranslator.java +++ b/underlay/src/main/java/bio/terra/tanagra/query/sql/translator/ApiTranslator.java @@ -228,7 +228,7 @@ default String booleanAndOrFilterSql( } default String booleanNotFilterSql(String subFilterSql) { - return "NOT " + subFilterSql; + return "NOT (" + subFilterSql + ")"; } default String logicalOperatorSql(BooleanAndOrFilter.LogicalOperator operator) { diff --git a/underlay/src/test/java/bio/terra/tanagra/query/bigquery/sqlbuilding/filter/BQBooleanLogicFilterTest.java b/underlay/src/test/java/bio/terra/tanagra/query/bigquery/sqlbuilding/filter/BQBooleanLogicFilterTest.java index 54c5f5bea..a30b5cda5 100644 --- a/underlay/src/test/java/bio/terra/tanagra/query/bigquery/sqlbuilding/filter/BQBooleanLogicFilterTest.java +++ b/underlay/src/test/java/bio/terra/tanagra/query/bigquery/sqlbuilding/filter/BQBooleanLogicFilterTest.java @@ -63,6 +63,39 @@ void booleanNotFilter() throws IOException { assertSqlMatchesWithTableNameOnly("booleanNotFilter", listQueryResult.getSql(), table); } + @Test + void booleanNotFilterWithNestedBooleanAndFilter() throws IOException { + Entity entity = underlay.getPrimaryEntity(); + AttributeFilter attributeFilter1 = + new AttributeFilter( + underlay, + entity, + entity.getAttribute("year_of_birth"), + BinaryOperator.NOT_EQUALS, + Literal.forInt64(1_956L)); + AttributeFilter attributeFilter2 = + new AttributeFilter( + underlay, + entity, + entity.getAttribute("gender"), + BinaryOperator.EQUALS, + Literal.forInt64(8_532L)); + BooleanNotFilter booleanNotFilter = + new BooleanNotFilter( + new BooleanAndOrFilter( + BooleanAndOrFilter.LogicalOperator.AND, + List.of(attributeFilter1, attributeFilter2))); + AttributeField simpleAttribute = + new AttributeField(underlay, entity, entity.getAttribute("year_of_birth"), false); + ListQueryResult listQueryResult = + bqQueryRunner.run( + ListQueryRequest.dryRunAgainstIndexData( + underlay, entity, List.of(simpleAttribute), booleanNotFilter, null, null)); + BQTable table = underlay.getIndexSchema().getEntityMain(entity.getName()).getTablePointer(); + assertSqlMatchesWithTableNameOnly( + "booleanNotFilterWithNestedBooleanAndFilter", listQueryResult.getSql(), table); + } + @Test void booleanAndOrFilterWithSwapFields() throws IOException { // e.g. SELECT occurrence BOOLEAN LOGIC FILTER ON condition (foreign key is on the occurrence diff --git a/underlay/src/test/java/bio/terra/tanagra/query/sql/ApiTranslatorTest.java b/underlay/src/test/java/bio/terra/tanagra/query/sql/ApiTranslatorTest.java index 41b5e369d..724a25609 100644 --- a/underlay/src/test/java/bio/terra/tanagra/query/sql/ApiTranslatorTest.java +++ b/underlay/src/test/java/bio/terra/tanagra/query/sql/ApiTranslatorTest.java @@ -259,7 +259,7 @@ void booleanNotFilter() { apiTranslator.binaryFilterSql( field, BinaryOperator.LESS_THAN_OR_EQUAL, val, tableAlias, sqlParams); String sql = apiTranslator.booleanNotFilterSql(subFilterSql); - assertEquals("NOT tableAlias.columnName <= @val0", sql); + assertEquals("NOT (tableAlias.columnName <= @val0)", sql); assertEquals(ImmutableMap.of("val0", val), sqlParams.getParams()); } diff --git a/underlay/src/test/resources/sql/BQBooleanLogicFilterTest/booleanNotFilter.sql b/underlay/src/test/resources/sql/BQBooleanLogicFilterTest/booleanNotFilter.sql index d43c03989..3bc73235c 100644 --- a/underlay/src/test/resources/sql/BQBooleanLogicFilterTest/booleanNotFilter.sql +++ b/underlay/src/test/resources/sql/BQBooleanLogicFilterTest/booleanNotFilter.sql @@ -4,4 +4,4 @@ FROM ${ENT_person} WHERE - NOT year_of_birth != @val0 + NOT (year_of_birth != @val0) diff --git a/underlay/src/test/resources/sql/BQBooleanLogicFilterTest/booleanNotFilterWithNestedBooleanAndFilter.sql b/underlay/src/test/resources/sql/BQBooleanLogicFilterTest/booleanNotFilterWithNestedBooleanAndFilter.sql new file mode 100644 index 000000000..82b40f1bb --- /dev/null +++ b/underlay/src/test/resources/sql/BQBooleanLogicFilterTest/booleanNotFilterWithNestedBooleanAndFilter.sql @@ -0,0 +1,8 @@ + + SELECT + year_of_birth + FROM + ${ENT_person} + WHERE + NOT ((year_of_birth != @val0) + AND (gender = @val1)) diff --git a/underlay/src/test/resources/sql/BQBooleanLogicFilterTest/booleanNotFilterWithSwapFields.sql b/underlay/src/test/resources/sql/BQBooleanLogicFilterTest/booleanNotFilterWithSwapFields.sql index d75c35bba..21afc1c31 100644 --- a/underlay/src/test/resources/sql/BQBooleanLogicFilterTest/booleanNotFilterWithSwapFields.sql +++ b/underlay/src/test/resources/sql/BQBooleanLogicFilterTest/booleanNotFilterWithSwapFields.sql @@ -4,7 +4,7 @@ FROM ${ENT_conditionOccurrence} WHERE - NOT condition IN (SELECT + NOT (condition IN (SELECT descendant FROM ${HAD_condition_default} @@ -12,4 +12,4 @@ ancestor = @val0 UNION ALL SELECT - @val1) + @val1))