Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect results of COUNT(DISTINCT) queries in MemSQL connector #9166

Closed
ebyhr opened this issue Sep 9, 2021 · 3 comments · Fixed by #9220
Closed

Incorrect results of COUNT(DISTINCT) queries in MemSQL connector #9166

ebyhr opened this issue Sep 9, 2021 · 3 comments · Fixed by #9220
Labels
bug Something isn't working correctness

Comments

@ebyhr
Copy link
Member

ebyhr commented Sep 9, 2021

TestMemSqlConnectorTest.testCountDistinctWithStringTypes looks not working.

Error:  Tests run: 268, Failures: 1, Errors: 0, Skipped: 33, Time elapsed: 465.575 s <<< FAILURE! - in TestSuite
Error:  io.trino.plugin.memsql.TestMemSqlConnectorTest.testCountDistinctWithStringTypes  Time elapsed: 1.651 s  <<< FAILURE!
java.lang.AssertionError: 
[Rows] 
Expecting:
  <(4)>
to contain exactly in any order:
  <[(7)]>
elements not found:
  <(7)>
and elements not expected:
  <(4)>

	at io.trino.sql.query.QueryAssertions$QueryAssert.lambda$matches$2(QueryAssertions.java:339)
	at io.trino.sql.query.QueryAssertions$QueryAssert.matches(QueryAssertions.java:326)
	at io.trino.sql.query.QueryAssertions$QueryAssert.matches(QueryAssertions.java:321)
	at io.trino.plugin.jdbc.BaseJdbcConnectorTest.testCountDistinctWithStringTypes(BaseJdbcConnectorTest.java:478)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:104)
	at org.testng.internal.Invoker.invokeMethod(Invoker.java:645)
	at org.testng.internal.Invoker.invokeTestMethod(Invoker.java:851)
	at org.testng.internal.Invoker.invokeTestMethods(Invoker.java:1177)
	at org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:129)
	at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:112)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:829)

Relates to #8562
cc: @findepi @losipiuk @alexjo2144

@hashhar
Copy link
Member

hashhar commented Sep 9, 2021

Looks like a correctness issue? It's not isolated to tests?

@ebyhr
Copy link
Member Author

ebyhr commented Sep 9, 2021

As far as I confirmed, this is not isolated to tests. Labeled bug & correctness.

MemSQL

memsql> select c1 from tpch.test_agg order by 1;
+------+
| c1   |
+------+
|      |
|  a   |
|  b   |
| A    |
| a    |
| a    |
| b    |
| b    |
| B    |
+------+
9 rows in set (0.01 sec)

memsql>
memsql> select count(distinct c1) from tpch.test_agg;
+--------------------+
| count(distinct c1) |
+--------------------+
|                  5 |
+--------------------+
1 row in set (0.02 sec)

memsql> select distinct c1 from tpch.test_agg order by 1;
+------+
| c1   |
+------+
|      |
|  a   |
|  b   |
| A    |
| b    |
+------+

PostgreSQL

tpch=# select * from tpch.test_agg order by 1;
 c1
-----

 a
 a
  a
 A
 b
 b
  b
 B
(9 rows)

tpch=#
tpch=# select count(distinct c1) from tpch.test_agg;
 count
-------
     7

tpch=# select distinct c1 from tpch.test_agg order by 1;
 c1
-----

 a
  a
 A
 b
  b
 B
(7 rows)

Trino

trino> select count(distinct c1) from mysql.tpch.test_agg order by 1;
 _col0
-------
     7

trino> select count(distinct c1) from postgresql.tpch.test_agg order by 1;
 _col0
-------
     7

trino> select count(distinct c1) from memsql.tpch.test_agg order by 1;
 _col0
-------
     5

trino> select count(distinct c1) from memsql.tpch.test_agg where rand() = 42 or c1 is not null order by 1;
 _col0
-------
     7

@ebyhr ebyhr added bug Something isn't working correctness labels Sep 9, 2021
@ebyhr ebyhr changed the title TestMemSqlConnectorTest.testCountDistinctWithStringTypes is very flaky Fix pushdown of count aggregation with distinct in MemSQL connector Sep 9, 2021
@hashhar hashhar changed the title Fix pushdown of count aggregation with distinct in MemSQL connector Incorrect results of COUNT(DISTINCT) queries in MemSQL connector Sep 12, 2021
@hashhar
Copy link
Member

hashhar commented Sep 12, 2021

A fix is implemented in #9220. PTAL.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctness
Development

Successfully merging a pull request may close this issue.

2 participants