[CALCITE-2327] In 3 valued logic mode (b and not b) may not be simplified to false by kgyrtkirk · Pull Request #707 · apache/calcite

kgyrtkirk · 2018-05-30T06:27:03Z

No description provided.

…fied to false Simplifying "b and not b" to false is incorrect if b is null.

jcamachor · 2018-05-31T20:51:32Z

-      final List<RexNode> terms2 = RelOptUtil.conjunctions(notDisjunction);
-      if (terms.containsAll(terms2)) {
-        return rexBuilder.makeLiteral(false);
+    if (unknownAsFalse) {


This method should only be called when unknownAsFalse is false, right? It is very confusing that this method checks the value of this boolean, given that we should only be in it if unknownAsFalse is false.
It would be more clear to change the boolean value before calling simplifyAnd, that would fix the issue.

Just to make sure my comment is understood: since we have two variants for the method, ideal solution is to prevent any method from calling simplifyAnd2 if unknownAsFalse is true.

correct; these duplicate methods are not making it easier...
I'll removed this block

…able x x AND NOT(x) ==> FALSE AND x IS NULL closes apache#707

[CALCITE-2462] RexProgramTest: replace nullLiteral->nullInt, unknownLiteral->nullBool for brevity Update Michael Mior's affiliation Site: Update Julian Hyde's affiliation [CALCITE-2480] NameSet.contains wrongly returns false when element in set is upper-case and seek is lower-case Implement toString, hashCode and equals for NameSet, NameMap and NameMultimap; and add tests. Fix some cosmetic issues in other code, such as missing newline at end of file. [CALCITE-2467] Upgrade owasp-dependency-check maven plugin to 3.3.1 [CALCITE-2481] NameSet assumes lower-case characters have greater codes, which does not hold for certain characters Includes EquivalenceSet, a nice general-purpose class to compute reflexive, symmetric, transitive closure. [CALCITE-2271] Join of two views with window aggregates produces incorrect results or NPE Avoid NPE in BlockBuilder.append when empty variable initializer is used closes apache#673 [CALCITE-2327] Avoid simplification of x AND NOT(x) to false for nullable x x AND NOT(x) ==> FALSE AND x IS NULL closes apache#707 [CALCITE-311] Added a test-case for filter after window aggregate Filters should not be pushed through window aggregate since it might impact results. [CALCITE-2494] RexFieldAccess should implement equals/hashCode [CALCITE-2327] RexSimplify: AND(x, y, NOT(y)) ==> AND(x, null, IS NULL(y)) [CALCITE-2497] Update Janino version to 3.0.9 [CALCITE-2495] Support encoded URLs in calcite.util.Source, and use it for URL->File conversion in tests For the record: 1) URL.getPath() produces %20, so it is added to forbidden signatures 2) Paths.get(url.toURI()).toFile() almost works, however it fails with URL is not hierarchical for new URL("file:test.java") 3) new File(URL.toURI() is worse than apache#2 4) URLDecoder must not be used to decode %20, since it will convert + to spaces as well, thus it will corrupt test.c++ filenames 5) It looks like url.toURI().getSchemeSpecificPart()) properly handles "opaque" URIs (which are relative file:test.java kind of URLs) 6) file:/test is relative for Windows, and absolute would be like file:c:/test MockCatalogReader is used in testing, so cache should be disabled there to avoid thread conflicts and/or stale results Support AND, OR, COALESCE, IS...DISTINCT in RexUtil#op [CALCITE-2505] Add ignored test for fail in RexSimplify with IS DISTINCT FROM Simplification of isFalse(isNotDistinctFrom(vBool(0), vBool(1))) causes AssertionError: wrong operand count 1 for IS DISTINCT FROM [CALCITE-2506] Add ignored test for fail in RexSimplify with coalesce coalesce(unaryPlus(nullInt), unaryPlus(vInt())) fails with result mismatch: when applied to {?0.int0=-1}, COALESCE(+(null), +(?0.int0)) yielded -1, and +(null) yielded NULL [CALCITE-2483] Druid adapter, when querying Druid segment metadata, throws when row number is larger than Integer.MAX_VALUE (Hongze Zhang) Close apache#799 [CALCITE-2469] RexSimplify should optimize '(NOT x) IS NULL' to 'x IS NULL' (pengzhiwei) Previously it optimized '(NOT x) IS NULL' to 'x IS NOT NULL', which is wrong. Generalize the above, to simplify 'f(x) IS NULL' to 'x IS NULL' for any operator 'f' that is known to be strong. (Julian Hyde) Close apache#796 [CALCITE-1026] Allow models in YAML format Make JSON/YAML mappers static final. Add tests to check model inline: json and yaml and to check model uri to hson/yaml files. Close apache#791 [CALCITE-2486] Upgrade Apache parent POM to version 21 Also upgrade commons-dbcp2 to 2.5.0, commons-lang3 to 3.8, commons-pool to 2.6.0, esri-geometry-api to 2.2.0, h2 to 1.4.197, hsqldb to 2.4.1, httpclient to 2.5.6, httpcore to 4.4.10, java-diff to 1.1.2, jsoup to 1.11.3, maven-dependency-plugin to 3.1.1, maven-enforcer-plugin to 3.0.0-M2, mockito to 2.21.0. [CALCITE-2470] In RelBuilder, project method should combine expressions if the underlying node is a Project Add RelBuilder.shouldMergeProject() to allow sub-classes to disable merging. Improve the message given by CompositeMatcher when match fails. When RelStructuredTypeFlattener rewrites a RexInputRef be sure to use the field's new type. (It might have strengthened from say INTEGER to INTEGER NOT NULL.) Add a test case inspired by Drill (it passes in Calcite, but I gather it fails in Drill). [CALCITE-1026] Fix ModelTest#testYamlFileDetection when source folder has spaces [CALCITE-1026] Remove unused import [CALCITE-2520] Make SparkHandlerImpl#compile silent by default, print code in calcite.debug=true mode only [CALCITE-2519] Silence ERROR logs from CalciteException, SqlValidatorException during tests Use threadCount=1/perCoreThreadCount=false to disable parallel test execution at Travis Previous option -Dsurefire.parallel= made JUnit to log just CalciteSuite details, thus the test looked like a stuck one fixes apache#816 Skip second Checkstyle execution during Travis CI build Checkstyle is validated during mvn install part, so there's no need to repeat that [CALCITE-2491] Refactor NameSet, NameMap, and NameMultimap Collections are thread-safe when used in read-only scenarios. Name*.immutableCopyOf are always thread-safe as writes are prohibited :) fixes apache#815 [CALCITE-2512] Move StreamTest#ROW_GENERATOR to Table.scan().iterator to make it not shared between threads (Sergey Nuyanzin) fixes apache#813 [CALCITE-2498] fix bug when geode adapter quotes booleans as strings (Andrei Sereda) GeodeFilter was incorrectly quoting boolean literals as SQL strings ('true' instead of true) fixes apache#809 [CALCITE-2522] Remove e.printStackTrace() from CalciteAssert#returns [CALCITE-2514] Add SqlIdentifier conversion to ITEM operator for dynamic tables in ExtendedExpander (Arina Ielchiieva) Close apache#814 Following [CALCITE-2469] simplify "f(x, y) IS NULL" to "x IS NULL OR y IS NULL" if "f" is strong Previously we did not use OR, which was wrong. [CALCITE-2418] Remove matchRecognize field of SqlSelect [CALCITE-2433] SqlAdvisor: support configurable quoting characters [CALCITE-2434] SqlAdvisor: support hints for nested tables/schemas [CALCITE-2473] SqlAdvisor: support -- comments [CALCITE-2474] SqlAdvisor: avoid NPE in lookupFromHints where FROM is empty [CALCITE-2475] SqlAdvisor: support MINUS [CALCITE-2476] SqlAdvisor: produce hints when sub-query with * is present in query simplifySql produced 0 AS "*", and it caused the SQL to fail validation. The fix is to disable transformation of expressions to "0 as ...", so it keeps select * as is. [CALCITE-2477] SqlAdvisor: scalar sub-query support [CALCITE-2478] SqlAdvisor: purge from_clause when _suggest_ token is located in one of the from sub-queries [CALCITE-2479] SqlAdvisor: automatically quote identifiers that look like SQL keywords [CALCITE-2484] Move dynamic tests to a separate class like SqlValidatorDynamicTest, and avoid reuse of MockCatalogReaderDynamic [CALCITE-2484] Add SqlValidatorDynamicTest to CalciteSuite [CALCITE-2521] Guard RelMetadataTest#testMetadataHandlerCacheLimit with CalciteAssert.ENABLE_SLOW The test does not reproduce the original bug, and it is very slow to execute. [CALCITE-2523] Guard PartiallyOrderedSetTest#testPosetBitsLarge with CalciteAssert.ENABLE_SLOW Reduce HepPlannerTest#testRuleApplyCount complexity [CALCITE-2543] Upgrade SQLLine to 1.5.0 [CALCITE-2112] Add Maven wrapper for Calcite (Ratandeep S. Ratti) Update doc, remove maven jar; add jar to .gitignore (Julian Hyde) Use TLS 1.2 on windows (Sergey Nuyanzin) [CALCITE-2412] Add Windows CI via AppVeyor (Sergey Nuyanzin) fixes apache#757

[CALCITE-2462] RexProgramTest: replace nullLiteral->nullInt, unknownLiteral->nullBool for brevity Update Michael Mior's affiliation Site: Update Julian Hyde's affiliation [CALCITE-2480] NameSet.contains wrongly returns false when element in set is upper-case and seek is lower-case Implement toString, hashCode and equals for NameSet, NameMap and NameMultimap; and add tests. Fix some cosmetic issues in other code, such as missing newline at end of file. [CALCITE-2467] Upgrade owasp-dependency-check maven plugin to 3.3.1 [CALCITE-2481] NameSet assumes lower-case characters have greater codes, which does not hold for certain characters Includes EquivalenceSet, a nice general-purpose class to compute reflexive, symmetric, transitive closure. [CALCITE-2271] Join of two views with window aggregates produces incorrect results or NPE Avoid NPE in BlockBuilder.append when empty variable initializer is used closes apache#673 [CALCITE-2327] Avoid simplification of x AND NOT(x) to false for nullable x x AND NOT(x) ==> FALSE AND x IS NULL closes apache#707 [CALCITE-311] Added a test-case for filter after window aggregate Filters should not be pushed through window aggregate since it might impact results. [CALCITE-2494] RexFieldAccess should implement equals/hashCode [CALCITE-2327] RexSimplify: AND(x, y, NOT(y)) ==> AND(x, null, IS NULL(y)) [CALCITE-2497] Update Janino version to 3.0.9 [CALCITE-2495] Support encoded URLs in calcite.util.Source, and use it for URL->File conversion in tests For the record: 1) URL.getPath() produces %20, so it is added to forbidden signatures 2) Paths.get(url.toURI()).toFile() almost works, however it fails with URL is not hierarchical for new URL("file:test.java") 3) new File(URL.toURI() is worse than apache#2 4) URLDecoder must not be used to decode %20, since it will convert + to spaces as well, thus it will corrupt test.c++ filenames 5) It looks like url.toURI().getSchemeSpecificPart()) properly handles "opaque" URIs (which are relative file:test.java kind of URLs) 6) file:/test is relative for Windows, and absolute would be like file:c:/test MockCatalogReader is used in testing, so cache should be disabled there to avoid thread conflicts and/or stale results Support AND, OR, COALESCE, IS...DISTINCT in RexUtil#op [CALCITE-2505] Add ignored test for fail in RexSimplify with IS DISTINCT FROM Simplification of isFalse(isNotDistinctFrom(vBool(0), vBool(1))) causes AssertionError: wrong operand count 1 for IS DISTINCT FROM [CALCITE-2506] Add ignored test for fail in RexSimplify with coalesce coalesce(unaryPlus(nullInt), unaryPlus(vInt())) fails with result mismatch: when applied to {?0.int0=-1}, COALESCE(+(null), +(?0.int0)) yielded -1, and +(null) yielded NULL [CALCITE-2483] Druid adapter, when querying Druid segment metadata, throws when row number is larger than Integer.MAX_VALUE (Hongze Zhang) Close apache#799 [CALCITE-2469] RexSimplify should optimize '(NOT x) IS NULL' to 'x IS NULL' (pengzhiwei) Previously it optimized '(NOT x) IS NULL' to 'x IS NOT NULL', which is wrong. Generalize the above, to simplify 'f(x) IS NULL' to 'x IS NULL' for any operator 'f' that is known to be strong. (Julian Hyde) Close apache#796 [CALCITE-1026] Allow models in YAML format Make JSON/YAML mappers static final. Add tests to check model inline: json and yaml and to check model uri to hson/yaml files. Close apache#791 [CALCITE-2486] Upgrade Apache parent POM to version 21 Also upgrade commons-dbcp2 to 2.5.0, commons-lang3 to 3.8, commons-pool to 2.6.0, esri-geometry-api to 2.2.0, h2 to 1.4.197, hsqldb to 2.4.1, httpclient to 2.5.6, httpcore to 4.4.10, java-diff to 1.1.2, jsoup to 1.11.3, maven-dependency-plugin to 3.1.1, maven-enforcer-plugin to 3.0.0-M2, mockito to 2.21.0. [CALCITE-2470] In RelBuilder, project method should combine expressions if the underlying node is a Project Add RelBuilder.shouldMergeProject() to allow sub-classes to disable merging. Improve the message given by CompositeMatcher when match fails. When RelStructuredTypeFlattener rewrites a RexInputRef be sure to use the field's new type. (It might have strengthened from say INTEGER to INTEGER NOT NULL.) Add a test case inspired by Drill (it passes in Calcite, but I gather it fails in Drill). [CALCITE-1026] Fix ModelTest#testYamlFileDetection when source folder has spaces [CALCITE-1026] Remove unused import [CALCITE-2520] Make SparkHandlerImpl#compile silent by default, print code in calcite.debug=true mode only [CALCITE-2519] Silence ERROR logs from CalciteException, SqlValidatorException during tests Use threadCount=1/perCoreThreadCount=false to disable parallel test execution at Travis Previous option -Dsurefire.parallel= made JUnit to log just CalciteSuite details, thus the test looked like a stuck one fixes apache#816 Skip second Checkstyle execution during Travis CI build Checkstyle is validated during mvn install part, so there's no need to repeat that [CALCITE-2491] Refactor NameSet, NameMap, and NameMultimap Collections are thread-safe when used in read-only scenarios. Name*.immutableCopyOf are always thread-safe as writes are prohibited :) fixes apache#815 [CALCITE-2512] Move StreamTest#ROW_GENERATOR to Table.scan().iterator to make it not shared between threads (Sergey Nuyanzin) fixes apache#813 [CALCITE-2498] fix bug when geode adapter quotes booleans as strings (Andrei Sereda) GeodeFilter was incorrectly quoting boolean literals as SQL strings ('true' instead of true) fixes apache#809 [CALCITE-2522] Remove e.printStackTrace() from CalciteAssert#returns [CALCITE-2514] Add SqlIdentifier conversion to ITEM operator for dynamic tables in ExtendedExpander (Arina Ielchiieva) Close apache#814 Following [CALCITE-2469] simplify "f(x, y) IS NULL" to "x IS NULL OR y IS NULL" if "f" is strong Previously we did not use OR, which was wrong. [CALCITE-2418] Remove matchRecognize field of SqlSelect [CALCITE-2433] SqlAdvisor: support configurable quoting characters [CALCITE-2434] SqlAdvisor: support hints for nested tables/schemas [CALCITE-2473] SqlAdvisor: support -- comments [CALCITE-2474] SqlAdvisor: avoid NPE in lookupFromHints where FROM is empty [CALCITE-2475] SqlAdvisor: support MINUS [CALCITE-2476] SqlAdvisor: produce hints when sub-query with * is present in query simplifySql produced 0 AS "*", and it caused the SQL to fail validation. The fix is to disable transformation of expressions to "0 as ...", so it keeps select * as is. [CALCITE-2477] SqlAdvisor: scalar sub-query support [CALCITE-2478] SqlAdvisor: purge from_clause when _suggest_ token is located in one of the from sub-queries [CALCITE-2479] SqlAdvisor: automatically quote identifiers that look like SQL keywords [CALCITE-2484] Move dynamic tests to a separate class like SqlValidatorDynamicTest, and avoid reuse of MockCatalogReaderDynamic [CALCITE-2484] Add SqlValidatorDynamicTest to CalciteSuite [CALCITE-2521] Guard RelMetadataTest#testMetadataHandlerCacheLimit with CalciteAssert.ENABLE_SLOW The test does not reproduce the original bug, and it is very slow to execute. [CALCITE-2523] Guard PartiallyOrderedSetTest#testPosetBitsLarge with CalciteAssert.ENABLE_SLOW Reduce HepPlannerTest#testRuleApplyCount complexity [CALCITE-2543] Upgrade SQLLine to 1.5.0 [CALCITE-2112] Add Maven wrapper for Calcite (Ratandeep S. Ratti) Update doc, remove maven jar; add jar to .gitignore (Julian Hyde) Use TLS 1.2 on windows (Sergey Nuyanzin) [CALCITE-2412] Add Windows CI via AppVeyor (Sergey Nuyanzin) fixes apache#757 [CALCITE-2505] Fix assertion error when simplifying is [not] distinct expressions (Haisheng Yuan) Sample expression that caused failure: isFalse(isNotDistinctFrom(vBool(0), vBool(1))) closes apache#828 Increase timeout for Cassandra daemon startup for CassandraAdapterTest Remove redundant 'new' expression in constant array creation add simplifyStrong for generic simplification for operand with null add comment for policy might be null make policy() return AS_IS by default remove unused null test simplify duplication code remove unused empty line fix duplicate call of simplify operand add comment for simplifyStrong

…able x x AND NOT(x) ==> FALSE AND x IS NULL closes apache#707

…able x x AND NOT(x) ==> FALSE AND x IS NULL closes apache#707 Change-Id: I723fc8b71b0df71ef8fa9d87c4daafdc0535811c

kgyrtkirk added 2 commits May 30, 2018 08:33

[CALCITE-2327] In 3 valued logic mode (b and not b) may not be simpli…

758ccba

…fied to false Simplifying "b and not b" to false is incorrect if b is null.

satisfiability is essentially withUnknownAsFalse(true)

005642e

kgyrtkirk force-pushed the 2327-3val-notb branch from a5c8e4a to 005642e Compare May 30, 2018 06:35

jcamachor requested changes May 31, 2018

View reviewed changes

remove block

7d1270e

asfgit force-pushed the master branch from 705d388 to 0204f28 Compare July 9, 2018 16:16

vlsi added a commit to vlsi/calcite that referenced this pull request Aug 28, 2018

[CALCITE-2327] Avoid simplification of x AND NOT(x) to false for null…

00a205f

…able x x AND NOT(x) ==> FALSE AND x IS NULL closes apache#707

vlsi mentioned this pull request Aug 28, 2018

[CALCITE-2327] Avoid simplification of x AND NOT(x) to false for nullable x #804

Merged

vlsi added a commit to vlsi/calcite that referenced this pull request Aug 28, 2018

[CALCITE-2327] Avoid simplification of x AND NOT(x) to false for null…

6fcd754

…able x x AND NOT(x) ==> FALSE AND x IS NULL closes apache#707

vlsi added a commit to vlsi/calcite that referenced this pull request Aug 28, 2018

[CALCITE-2327] Avoid simplification of x AND NOT(x) to false for null…

faeab72

…able x x AND NOT(x) ==> FALSE AND x IS NULL closes apache#707

vlsi added a commit to vlsi/calcite that referenced this pull request Aug 28, 2018

[CALCITE-2327] Avoid simplification of x AND NOT(x) to false for null…

a074f19

…able x x AND NOT(x) ==> FALSE AND x IS NULL closes apache#707

vlsi added a commit to vlsi/calcite that referenced this pull request Aug 28, 2018

[CALCITE-2327] Avoid simplification of x AND NOT(x) to false for null…

141781b

…able x x AND NOT(x) ==> FALSE AND x IS NULL closes apache#707

asfgit closed this in #804 Aug 28, 2018

F21 pushed a commit to F21/calcite that referenced this pull request Jan 3, 2019

[CALCITE-2327] Avoid simplification of x AND NOT(x) to false for null…

e2de927

…able x x AND NOT(x) ==> FALSE AND x IS NULL closes apache#707

wangxlong pushed a commit to wangxlong/calcite that referenced this pull request Feb 13, 2020

[CALCITE-2327] Avoid simplification of x AND NOT(x) to false for null…

d641d15

…able x x AND NOT(x) ==> FALSE AND x IS NULL closes apache#707

Navigation Menu

Uh oh!

[CALCITE-2327] In 3 valued logic mode (b and not b) may not be simplified to false - #707

[CALCITE-2327] In 3 valued logic mode (b and not b) may not be simplified to false#707
kgyrtkirk wants to merge 3 commits into
apache:masterfrom
kgyrtkirk:2327-3val-notb

kgyrtkirk commented May 30, 2018

Uh oh!

jcamachor May 31, 2018

Uh oh!

jcamachor May 31, 2018

Uh oh!

kgyrtkirk Jun 1, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

kgyrtkirk commented May 30, 2018

Uh oh!

jcamachor May 31, 2018

Choose a reason for hiding this comment

Uh oh!

jcamachor May 31, 2018

Choose a reason for hiding this comment

Uh oh!

kgyrtkirk Jun 1, 2018

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants