Fix case insensitive predicate pushdown for MySQL, MemSQL and SQL Server #6753

kokosing · 2021-01-28T21:12:09Z

hashhar

LGTM % "Allow to reuse few testing servers" commit.

Reusing containers speeds up tests but we need to be sure that all existing tests clean up properly after themselves to avoid tests masking each other's failures. The most common thing I can think of is GRANTing permissions to some users.

The comments I've added are for intermediate commits which get removed in future commits and as such they are only nits.

hashhar · 2021-01-29T05:46:48Z

plugin/trino-memsql/src/test/java/io/trino/plugin/memsql/TestMemSqlDistributedQueries.java

@@ -120,6 +120,11 @@ protected TestTable createTableWithDefaultColumns()
            return Optional.empty();
        }

+        if (typeName.contains("char(1)")) {
+            // https://github.com/trinodb/trino/issues/6746


Please add the same link to the varchar branch above.

No. Varchar above do not use case insensitive data. So it fails for the reason in TODO.

I did not understand the explanation. Do you mean that MemSQL's varchar(1) is case sensitive (by default), while char(1) is not (by default)?

No. varchar above (is not varchar(1)) is testing UTF-8

hashhar · 2021-01-29T05:47:23Z

plugin/trino-mysql/src/test/java/io/trino/plugin/mysql/TestMySqlDistributedQueries.java

@@ -139,6 +139,16 @@ protected boolean isColumnNameRejected(Exception exception, String columnName, b
            return Optional.empty();
        }

+        if (typeName.equals("char(1)")) {


Can these be merged into a single if since the cause is same?

It could be, but I fix them one by one in separate commits.

hashhar · 2021-01-29T05:47:48Z

...trino-sqlserver/src/test/java/io/trino/plugin/sqlserver/TestSqlServerDistributedQueries.java

@@ -103,6 +103,11 @@ protected TestTable createTableWithDefaultColumns()
            return Optional.empty();
        }

+        if (typeName.contains("char(1)")) {


Why is varchar not skipped here?

it is. Notice contains.

Oh - I did not notice that. Please change to equals || equals :)

Or - actually - NVM - you get rid of that anway.

losipiuk

Fixes up to Close testing servers after QueryRunner good. Consider extracting separete PR so we merge already.

plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/PredicatePushdownController.java

losipiuk · 2021-01-29T09:39:22Z

plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/PredicatePushdownController.java

+
+        DomainPushdownResult disabledPredicatePushdown = new DomainPushdownResult(Domain.all(domain.getType()), domain);
+
+        if (!domain.getValues().isDiscreteSet() && domain.getValues().complement().isDiscreteSet()) {


This is already covered by the for below. Right?

No. The below handles ranges like -inf, value or value, +inf

The one below handles any case where we have range which looks like ..., x) or (x,.... Which is also true for case which is covered by this if. Unless I am missing something.

findepi · 2021-01-29T09:45:10Z

@kokosing i very much want to review this. Currently this is 14 commits though.
Is there any way to reduce scope, eg review some simple refactors separately? Or you're concerned those refactors do not make sense on its own, without subsequent changes?

Also, for benefit of everyone involved here, can you please add description to the PR?

testing/trino-testing/src/main/java/io/trino/testing/AbstractTestDistributedQueries.java

findepi · 2021-01-29T09:48:38Z

MySql, MemSql and SqlServer

BTW please fix the names: MySQL, MemSQL, SQL Server.

...ino-sqlserver/src/test/java/io/trino/plugin/sqlserver/TestSqlServerIntegrationSmokeTest.java

losipiuk

Commits reagarding predicatate pushdown changes mostly good. Minor comments. Thanks.

I did not review the changes regarding container reusability. No context on that.

findepi · 2021-01-29T09:53:59Z

testing/trino-testing/src/main/java/io/trino/testing/containers/TestContainers.java

+public final class TestContainers
+{
+    // Please turn it on locally so container will survive JVM reboot.
+    private static final boolean TESTCONTAINERS_REUSE_ENABLE = "true".equalsIgnoreCase(getenv("TESTCONTAINERS_REUSE_ENABLE"));


Boolean.getBoolean

Boolean.getBoolean

This is using system property, while I used env variable, like testcontainers did.

testing/trino-testing/src/main/java/io/trino/testing/containers/TestContainers.java

findepi · 2021-01-29T09:55:19Z

testing/trino-testing/src/main/java/io/trino/testing/containers/TestContainers.java

+public final class TestContainers
+{
+    // Please turn it on locally so container will survive JVM reboot.
+    private static final boolean TESTCONTAINERS_REUSE_ENABLE = "true".equalsIgnoreCase(getenv("TESTCONTAINERS_REUSE_ENABLE"));


REUSE_ENABLE -> ENABLE_REUSE or REUSE_ENABLED

Sorry, I cannot change this. This env variable is defined in testcontainers. I am just verifying it if it works as expected. Also it matches testcontainers config property testcontainers.reuse.enable.

testing/trino-testing/src/main/java/io/trino/testing/AbstractTestDistributedQueries.java

kokosing

@findepi @losipiuk CA, PTALA

kokosing · 2021-01-30T22:12:47Z

testing/trino-testing/src/main/java/io/trino/testing/containers/TestContainers.java

+public final class TestContainers
+{
+    // Please turn it on locally so container will survive JVM reboot.
+    private static final boolean TESTCONTAINERS_REUSE_ENABLE = "true".equalsIgnoreCase(getenv("TESTCONTAINERS_REUSE_ENABLE"));


Sorry, I cannot change this. This env variable is defined in testcontainers. I am just verifying it if it works as expected. Also it matches testcontainers config property testcontainers.reuse.enable.

kokosing · 2021-01-30T22:13:47Z

testing/trino-testing/src/main/java/io/trino/testing/containers/TestContainers.java

+public final class TestContainers
+{
+    // Please turn it on locally so container will survive JVM reboot.
+    private static final boolean TESTCONTAINERS_REUSE_ENABLE = "true".equalsIgnoreCase(getenv("TESTCONTAINERS_REUSE_ENABLE"));


Boolean.getBoolean

This is using system property, while I used env variable, like testcontainers did.

kokosing · 2021-01-30T22:23:46Z

plugin/trino-memsql/src/test/java/io/trino/plugin/memsql/TestMemSqlDistributedQueries.java

@@ -120,6 +120,11 @@ protected TestTable createTableWithDefaultColumns()
            return Optional.empty();
        }

+        if (typeName.contains("char(1)")) {
+            // https://github.com/trinodb/trino/issues/6746


No. Varchar above do not use case insensitive data. So it fails for the reason in TODO.

kokosing · 2021-01-30T22:24:11Z

plugin/trino-mysql/src/test/java/io/trino/plugin/mysql/TestMySqlDistributedQueries.java

@@ -139,6 +139,16 @@ protected boolean isColumnNameRejected(Exception exception, String columnName, b
            return Optional.empty();
        }

+        if (typeName.equals("char(1)")) {


It could be, but I fix them one by one in separate commits.

kokosing · 2021-01-30T22:24:32Z

...trino-sqlserver/src/test/java/io/trino/plugin/sqlserver/TestSqlServerDistributedQueries.java

@@ -103,6 +103,11 @@ protected TestTable createTableWithDefaultColumns()
            return Optional.empty();
        }

+        if (typeName.contains("char(1)")) {


it is. Notice contains.

plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/PredicatePushdownController.java

kokosing · 2021-01-30T22:39:07Z

plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/PredicatePushdownController.java

+
+        DomainPushdownResult disabledPredicatePushdown = new DomainPushdownResult(Domain.all(domain.getType()), domain);
+
+        if (!domain.getValues().isDiscreteSet() && domain.getValues().complement().isDiscreteSet()) {


No. The below handles ranges like -inf, value or value, +inf

...ino-sqlserver/src/test/java/io/trino/plugin/sqlserver/TestSqlServerIntegrationSmokeTest.java

losipiuk

LGTM

losipiuk · 2021-02-01T11:13:38Z

mind the red (I did not check if related)

findepi · 2021-02-01T12:10:13Z

@findepi @losipiuk CA, PTALA

@kokosing would it work for you if we got #6759 in, rebased this & and then reviewed?

ssheikin · 2021-02-01T12:20:48Z

plugin/trino-mysql/src/main/java/io/trino/plugin/mysql/MySqlClient.java

@@ -290,18 +291,21 @@ protected String getTableSchemaName(ResultSet resultSet)
                return Optional.of(decimalColumnMapping(createDecimalType(precision, max(decimalDigits, 0))));

            case Types.CHAR:
-                return Optional.of(defaultCharColumnMapping(typeHandle.getRequiredColumnSize()));
+                int columnSize = typeHandle.getRequiredColumnSize();


extract method as you did for LONGNVARCHAR

Extracted method is reused. I would prefer to keep as it is.

kokosing · 2021-02-01T13:42:19Z

@kokosing would it work for you if we got #6759 in, rebased this & and then reviewed?

@findepi Sure, also I would like to get #6776 in before rebasing. Reusing makes it nicer to work with MySQL connector.

kokosing · 2021-02-06T19:46:23Z

@findepi Rebased.

🤯 I can't believe my eyes. Postgres fails the tests.

kokosing · 2021-02-06T19:58:11Z

tpch=# select * from tpch.test_data_mapping_smoke_char_1__jg9rm order by value;
    row_id    | value
--------------+-------
 high value   | a
 sample value | A
 null value   |
(3 rows)

tpch=# select * from tpch.test_data_mapping_smoke_char_1__jg9rm where value < 'A';
   row_id   | value
------------+-------
 high value | a
(1 row)

tpch=# select * from tpch.test_data_mapping_smoke_char_1__jg9rm where value > 'A';
 row_id | value
--------+-------
(0 rows)

Postgres is case sensitive but it has different collation. Should CASE_INSENSITIVE_PUSHDOWN become default for char and varchar columns?

kokosing · 2021-02-06T20:00:43Z

More tests:

tpch=# select *, value = 'a' from tpch.test_data_mapping_smoke_char_1__chek9;
    row_id    | value | ?column?
--------------+-------+----------
 null value   |       |
 sample value | A     | f
 high value   | a     | t
(3 rows)

tpch=# select *, value = 'a' from tpch.test_data_mapping_smoke_char_1__chek9 order by value;
    row_id    | value | ?column?
--------------+-------+----------
 high value   | a     | t
 sample value | A     | f
 null value   |       |
(3 rows)

tpch=# select *, value = 'a' from tpch.test_data_mapping_smoke_char_1__chek9 order by value desc;
    row_id    | value | ?column?
--------------+-------+----------
 null value   |       |
 sample value | A     | f
 high value   | a     | t
(3 rows)

findepi · 2021-02-06T21:49:58Z

Postgres is case sensitive but it has different collation. Should CASE_INSENSITIVE_PUSHDOWN become default for char and varchar columns?

This is covered by #3645

findepi · 2021-02-08T12:06:09Z

plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/PredicatePushdownController.java

@@ -30,7 +30,7 @@
        }
        return new DomainPushdownResult(domain, Domain.all(domain.getType()));
    };
-    PredicatePushdownController PUSHDOWN_AND_KEEP = (session, domain) -> new DomainPushdownResult(
+    PredicatePushdownController CASE_INSENSITIVE_PUSHDOWN = (session, domain) -> new DomainPushdownResult(


Rename PredicatePushdownController to match semantic

This commit does not rename PredicatePushdownController

the PredicatePushdownController interface is type-agnostic and "case-insensitive" is applicable to char/varchar type only, so maybe:

CASE_INSENSITIVE_PUSHDOWN -> eg CASE_INSENSITIVE_CHARACTER_PUSHDOWN

testing/trino-testing/src/main/java/io/trino/testing/AbstractTestDistributedQueries.java

plugin/trino-memsql/src/test/java/io/trino/plugin/memsql/TestMemSqlDistributedQueries.java

findepi · 2021-02-08T12:17:18Z

plugin/trino-memsql/src/test/java/io/trino/plugin/memsql/TestMemSqlDistributedQueries.java

@@ -120,6 +120,11 @@ protected TestTable createTableWithDefaultColumns()
            return Optional.empty();
        }

+        if (typeName.contains("char(1)")) {
+            // https://github.com/trinodb/trino/issues/6746


I did not understand the explanation. Do you mean that MemSQL's varchar(1) is case sensitive (by default), while char(1) is not (by default)?

plugin/trino-mysql/src/main/java/io/trino/plugin/mysql/MySqlClient.java

findepi · 2021-02-08T12:28:13Z

plugin/trino-mysql/src/main/java/io/trino/plugin/mysql/MySqlClient.java

@@ -82,11 +82,11 @@
 import static io.trino.plugin.jdbc.PredicatePushdownController.FULL_PUSHDOWN;
 import static io.trino.plugin.jdbc.StandardColumnMappings.bigintColumnMapping;
 import static io.trino.plugin.jdbc.StandardColumnMappings.bigintWriteFunction;
+import static io.trino.plugin.jdbc.StandardColumnMappings.charReadFunction;


Case insensitive predicate pushdown for char type in MySQL

Can you please make it a sentence?

plugin/trino-mysql/src/main/java/io/trino/plugin/mysql/MySqlClient.java

plugin/trino-sqlserver/src/main/java/io/trino/plugin/sqlserver/SqlServerClient.java

findepi · 2021-02-08T12:35:52Z

plugin/trino-memsql/src/test/java/io/trino/plugin/memsql/TestMemSqlDistributedQueries.java

@@ -120,11 +120,6 @@ protected TestTable createTableWithDefaultColumns()
            return Optional.empty();
        }

-        if (typeName.contains("char(1)")) {


With the TODO removals being scattered across commits, it's harder to see how many of them remained.
Also, the code in all these connectors looks very repetitive, so I think you can safely squash up the commit adding tests and commits fixing the problem across connectors, without any downside on the reviewability.

kokosing

@losipiuk @findepi CA, PTAL

testing/trino-testing/src/main/java/io/trino/testing/AbstractTestDistributedQueries.java

kokosing · 2021-02-17T10:49:58Z

plugin/trino-memsql/src/test/java/io/trino/plugin/memsql/TestMemSqlDistributedQueries.java

@@ -120,6 +120,11 @@ protected TestTable createTableWithDefaultColumns()
            return Optional.empty();
        }

+        if (typeName.contains("char(1)")) {
+            // https://github.com/trinodb/trino/issues/6746


No. varchar above (is not varchar(1)) is testing UTF-8

kokosing · 2021-02-17T11:17:36Z

plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/PredicatePushdownController.java

+                    domain.simplify(getDomainCompactionThreshold(session)),
+                    domain);
+        }
+        return new DomainPushdownResult(Domain.all(domain.getType()), domain);


That requires field reordering.

plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/PredicatePushdownController.java

kokosing · 2021-02-17T11:22:38Z

...no-postgresql/src/test/java/io/trino/plugin/postgresql/TestPostgreSqlDistributedQueries.java

+        if (typeName.equals("time")
+                || typeName.equals("timestamp(3) with time zone")) {
+            return Optional.of(dataMappingTestSetup.asUnsupported());


kokosing · 2021-02-17T11:51:19Z

plugin/trino-mysql/src/main/java/io/trino/plugin/mysql/MySqlClient.java

@@ -290,18 +291,21 @@ protected String getTableSchemaName(ResultSet resultSet)
                return Optional.of(decimalColumnMapping(createDecimalType(precision, max(decimalDigits, 0))));

            case Types.CHAR:
-                return Optional.of(defaultCharColumnMapping(typeHandle.getRequiredColumnSize()));
+                int columnSize = typeHandle.getRequiredColumnSize();


Extracted method is reused. I would prefer to keep as it is.

findepi

Looks good overall.

Can you please changes as fixups?

testing/trino-testing/src/main/java/io/trino/testing/AbstractTestDistributedQueries.java

plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/PredicatePushdownController.java

plugin/trino-memsql/src/main/java/io/trino/plugin/memsql/MemSqlClient.java

...no-postgresql/src/test/java/io/trino/plugin/postgresql/TestPostgreSqlDistributedQueries.java

plugin/trino-sqlserver/src/main/java/io/trino/plugin/sqlserver/SqlServerClient.java

...ino-sqlserver/src/test/java/io/trino/plugin/sqlserver/TestSqlServerIntegrationSmokeTest.java

testing/trino-testing/src/main/java/io/trino/testing/AbstractTestDistributedQueries.java

kokosing · 2021-02-19T13:26:10Z

@findepi PTAL

losipiuk · 2021-02-22T10:10:47Z

testing/trino-testing/src/main/java/io/trino/testing/AbstractTestDistributedQueries.java

@@ -1203,23 +1203,23 @@ public void testDataMappingSmokeTest(DataMappingTestSetup dataMappingTestSetup)

        // without pushdown, i.e. test read data mapping
        assertQuery("SELECT row_id FROM " + tableName + " WHERE rand() = 42 OR value IS NULL", "VALUES 'null value'");
-        assertQuery("SELECT row_id FROM " + tableName + " WHERE rand() = 42 OR value IS NOT NULL", "VALUES ('sample value'), ('high value')");
+        assertQuery("SELECT row_id FROM " + tableName + " WHERE rand() = 42 OR value IS NOT NULL", "VALUES 'sample value', 'high value'");


Simplify testDataMappingSmokeTest can be extracted and merged already.

plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/StandardColumnMappings.java

plugin/trino-memsql/src/main/java/io/trino/plugin/memsql/MemSqlClient.java

plugin/trino-sqlserver/src/main/java/io/trino/plugin/sqlserver/SqlServerClient.java

...trino-cassandra/src/test/java/io/trino/plugin/cassandra/BaseCassandraDistributedQueries.java

findepi · 2021-02-22T10:47:28Z

...no-postgresql/src/test/java/io/trino/plugin/postgresql/TestPostgreSqlDistributedQueries.java

+    @Override
+    public void testCaseSensitiveDataMapping(DataMappingTestSetup dataMappingTestSetup)
+    {
+        throw new SkipException("https://github.com/trinodb/trino/issues/3645 - PostgreSQL has different collation than Trino");


can you please

assertThatThrowBy(super...) .hasMessage("something something");

before skipping?

In case insensitive predicate pushdown, consider values 'A' and 'a'. Both are equal in data source, but different in Trino. It was safe to pushdown `x = 'a'` or `x = 'A'` as both values where always returned and we could filter them out in Trino. But operators like `!=`, `<` or `>` may return no value at all, which is incorrect as Trino is case sensitive only today. Consider also a casse of `B` and `a`. In Trino `B` is lower than `a`, but if you compare them in remote data source which is casee insensitive then `B` or `b` are higher `A` or `b`. Considering the above we can only push down the predicate for case insensitive column only when `=` operator is used.

findepi · 2021-02-23T14:38:10Z

plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/PredicatePushdownController.java

+        }
+
+        // case insensitive predicate pushdown could return incorrect results for operators like `!=`, `<` or `>`
+        return DISABLE_PUSHDOWN.apply(session, domain);


As discussed offline, it may be reasonable to have a session toggle to unlock legacy unsafe, performant behavior -- especially for the cases where user knows it's safe (and we do not)

Sure, let me do it as separate PR.

kokosing · 2021-02-23T19:47:36Z

CI hit #7009

findepi · 2021-02-25T12:37:02Z

...n/trino-postgresql/src/test/java/io/trino/plugin/postgresql/TestPostgreSqlConnectorTest.java

+        try {
+            super.testCaseSensitiveDataMapping(dataMappingTestSetup);
+        }
+        catch (AssertionError ignored) {
+            // TODO https://github.com/trinodb/trino/issues/3645 - PostgreSQL has different collation than Trino
+            assertThatThrownBy(() -> super.testCaseSensitiveDataMapping(dataMappingTestSetup))
+                    .hasStackTraceContaining("not equal\nActual rows");
+        }


This is a long version of saying

// TODO https://github.com/trinodb/trino/issues/3645 - PostgreSQL has different collation than Trino

as we do not validate whether test passes or fails.

Also, this should throw SkippedException, instead of passing, so

// TODO https://github.com/trinodb/trino/issues/3645 - PostgreSQL has different collation than Trino throw new SkippedException("TODO");

Thanks, see: #7032

cla-bot bot added the cla-signed label Jan 28, 2021

kokosing requested review from findepi, losipiuk, ssheikin and skrzypo987 January 28, 2021 21:13

hashhar approved these changes Jan 29, 2021

View reviewed changes

losipiuk reviewed Jan 29, 2021

View reviewed changes

plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/PredicatePushdownController.java Outdated Show resolved Hide resolved

losipiuk reviewed Jan 29, 2021

View reviewed changes

testing/trino-testing/src/main/java/io/trino/testing/AbstractTestDistributedQueries.java Outdated Show resolved Hide resolved

losipiuk reviewed Jan 29, 2021

View reviewed changes

testing/trino-testing/src/main/java/io/trino/testing/AbstractTestDistributedQueries.java Outdated Show resolved Hide resolved

losipiuk reviewed Jan 29, 2021

View reviewed changes

...ino-sqlserver/src/test/java/io/trino/plugin/sqlserver/TestSqlServerIntegrationSmokeTest.java Show resolved Hide resolved

losipiuk reviewed Jan 29, 2021

View reviewed changes

findepi reviewed Jan 29, 2021

View reviewed changes

testing/trino-testing/src/main/java/io/trino/testing/AbstractTestDistributedQueries.java Outdated Show resolved Hide resolved

testing/trino-testing/src/main/java/io/trino/testing/AbstractTestDistributedQueries.java Outdated Show resolved Hide resolved

kokosing force-pushed the origin/master/270_case_chars branch from 6b59231 to 0a2908a Compare January 30, 2021 23:08

kokosing commented Jan 30, 2021

View reviewed changes

losipiuk reviewed Feb 1, 2021

View reviewed changes

...ino-sqlserver/src/test/java/io/trino/plugin/sqlserver/TestSqlServerIntegrationSmokeTest.java Show resolved Hide resolved

losipiuk reviewed Feb 1, 2021

View reviewed changes

...ino-sqlserver/src/test/java/io/trino/plugin/sqlserver/TestSqlServerIntegrationSmokeTest.java Show resolved Hide resolved

losipiuk approved these changes Feb 1, 2021

View reviewed changes

ssheikin reviewed Feb 1, 2021

View reviewed changes

kokosing force-pushed the origin/master/270_case_chars branch from 0a2908a to 15860e5 Compare February 6, 2021 19:45

kokosing force-pushed the origin/master/270_case_chars branch from 15860e5 to 0647256 Compare February 7, 2021 20:16

findepi requested changes Feb 8, 2021

View reviewed changes

kokosing force-pushed the origin/master/270_case_chars branch from 0647256 to d501959 Compare February 17, 2021 12:37

kokosing commented Feb 17, 2021

View reviewed changes

kokosing force-pushed the origin/master/270_case_chars branch from d501959 to 61d5d63 Compare February 17, 2021 12:47

findepi reviewed Feb 18, 2021

View reviewed changes

kokosing force-pushed the origin/master/270_case_chars branch from 61d5d63 to 58ab40b Compare February 19, 2021 13:24

findepi changed the title ~~Fix case insensitive predicate pushdown for MySql, MemSql and SqlServer~~ Fix case insensitive predicate pushdown for MySQL, MemSQL and SQL Server Feb 21, 2021

losipiuk reviewed Feb 22, 2021

View reviewed changes

losipiuk approved these changes Feb 22, 2021

View reviewed changes

findepi approved these changes Feb 22, 2021

View reviewed changes

kokosing force-pushed the origin/master/270_case_chars branch 3 times, most recently from 2e37870 to 80d11f4 Compare February 23, 2021 11:48

kokosing added 2 commits February 23, 2021 14:39

Simplify testDataMappingSmokeTest

db4e9c4

kokosing force-pushed the origin/master/270_case_chars branch from 80d11f4 to 9f04547 Compare February 23, 2021 13:39

findepi reviewed Feb 23, 2021

View reviewed changes

kokosing merged commit 2da5aa2 into trinodb:master Feb 23, 2021

kokosing deleted the origin/master/270_case_chars branch February 23, 2021 19:48

kokosing mentioned this pull request Feb 23, 2021

Release notes for 353 #6858

Closed

10 tasks

findepi reviewed Feb 25, 2021

View reviewed changes

findepi mentioned this pull request Mar 5, 2021

Add 353 release notes #7124

Closed

martint added this to the 353 milestone Mar 5, 2021


		DomainPushdownResult disabledPredicatePushdown = new DomainPushdownResult(Domain.all(domain.getType()), domain);

		if (!domain.getValues().isDiscreteSet() && domain.getValues().complement().isDiscreteSet()) {

Fix case insensitive predicate pushdown for MySQL, MemSQL and SQL Server #6753

Fix case insensitive predicate pushdown for MySQL, MemSQL and SQL Server #6753

Conversation

kokosing commented Jan 28, 2021 • edited Loading

hashhar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

losipiuk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

findepi commented Jan 29, 2021

findepi commented Jan 29, 2021

losipiuk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kokosing left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

losipiuk left a comment

Choose a reason for hiding this comment

losipiuk commented Feb 1, 2021

findepi commented Feb 1, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kokosing commented Feb 1, 2021 • edited Loading

kokosing commented Feb 6, 2021

kokosing commented Feb 6, 2021

kokosing commented Feb 6, 2021

findepi commented Feb 6, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kokosing left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

findepi left a comment

Choose a reason for hiding this comment

kokosing commented Feb 19, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kokosing commented Feb 23, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kokosing commented Jan 28, 2021 •

edited

Loading

kokosing commented Feb 1, 2021 •

edited

Loading