[multistage][test] add table expression tests #9817

walterddr · 2022-11-16T22:09:35Z

from PostgresQL doc section 7.2.2

agavra

lgtm, suggest adding two tests

agavra · 2022-11-16T23:03:37Z

pinot-query-runtime/src/test/resources/queries/TableExpressions.json

+    },
+    "queries": [
+      {
+        "sql": "SELECT strCol FROM {tbl1} GROUP BY strCol"


can we group by multiple columns?

can we group by a function call?

there's already a multi column group by but i added more

also added group by without agg with single/multi columns (distinct basically)

added function call on group by too. (alias will not be covered as they will be in the alias test cases)

codecov-commenter · 2022-11-16T23:17:47Z

Codecov Report

Merging #9817 (36cab68) into master (6ef4dfc) will decrease coverage by 28.56%.
The diff coverage is n/a.

@@              Coverage Diff              @@
##             master    #9817       +/-   ##
=============================================
- Coverage     63.44%   34.88%   -28.57%     
+ Complexity     5326      199     -5127     
=============================================
  Files          1960     1972       +12     
  Lines        105350   105703      +353     
  Branches      15960    15998       +38     
=============================================
- Hits          66840    36872    -29968     
- Misses        33589    65707    +32118     
+ Partials       4921     3124     -1797

Flag	Coverage Δ
integration1	`?`
integration2	`24.62% <ø> (+0.05%)`	⬆️
unittests1	`?`
unittests2	`15.75% <ø> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...ain/java/org/apache/pinot/spi/utils/LoopUtils.java	`0.00% <0.00%> (-100.00%)`	⬇️
...in/java/org/apache/pinot/spi/utils/BytesUtils.java	`0.00% <0.00%> (-100.00%)`	⬇️
...java/org/apache/pinot/spi/trace/BaseRecording.java	`0.00% <0.00%> (-100.00%)`	⬇️
...java/org/apache/pinot/spi/trace/NoOpRecording.java	`0.00% <0.00%> (-100.00%)`	⬇️
...ava/org/apache/pinot/spi/config/table/FSTType.java	`0.00% <0.00%> (-100.00%)`	⬇️
...ava/org/apache/pinot/spi/config/user/RoleType.java	`0.00% <0.00%> (-100.00%)`	⬇️
...ava/org/apache/pinot/spi/data/MetricFieldSpec.java	`0.00% <0.00%> (-100.00%)`	⬇️
...ava/org/apache/pinot/spi/stream/StreamMessage.java	`0.00% <0.00%> (-100.00%)`	⬇️
...java/org/apache/pinot/common/tier/TierFactory.java	`0.00% <0.00%> (-100.00%)`	⬇️
...a/org/apache/pinot/spi/config/table/TableType.java	`0.00% <0.00%> (-100.00%)`	⬇️
... and 1353 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

siddharthteotia · 2022-11-17T15:30:52Z

pinot-query-runtime/src/test/resources/queries/TableExpressions.json

+        ]
+      }
+    },
+    "queries": [


Should we also add the following ?

Non-correlated sub-query with HAVING

Without the subquery, we will first have to execute a query to find overall average and hardcode that value in HAVING clause of the second query.

SELECT group1, group2, AVG(col) AS groupAverage FROM FOO HAVING groupAverage > ( SELECT AVG(col) FROM FOO)

Similarly correlated sub-query with HAVING

SELECT group1, group2, AVG(col) AS groupAverage FROM FOO as OUTER HAVING groupAverage > ( SELECT AVG(col) FROM FOO WHERE group2 < OUTER.group2 )

I don't think correlated are supported yet though ?

correct correlate is not supported.

pinot-query-runtime/src/test/resources/queries/TableExpressions.json

siddharthteotia · 2022-11-17T15:40:12Z

pinot-query-runtime/src/test/resources/queries/TableExpressions.json

@@ -0,0 +1,93 @@
+{


High level question on table expression tests?

By table expressions do you mean CTEs ? CTEs create named queries using WITH afaik.

May be we can call these Subqueries.json to distinguish from CTEs ?

table expression as defined in postgresql 7.2 section. (e.g. all clauses that alters the behavior of a SQL after choosing the tables)
some of the sections we don't support is not included such as:

7.2.4. GROUPING SETS, CUBE, and ROLLUP

7.2.5. Window Function Processing

pinot-query-runtime/src/test/resources/queries/TableExpressions.json

siddharthteotia · 2022-11-17T16:31:22Z

pinot-query-runtime/src/test/resources/queries/TableExpressions.json

+      { "sql": "SELECT strCol, intCol FROM {tbl1} GROUP BY strCol, intCol" },
+      { "sql": "SELECT strCol, SUM(intCol) FROM {tbl1} GROUP BY strCol" },
+      { "sql": "SELECT strCol, b.strCol2, (sum(a.intCol) * b.intCol) AS colAlias FROM {tbl1} a INNER JOIN {tbl2} b ON a.strCol = b.strCol1 GROUP BY strCol, b.strCol2, b.intCol" },
+      { "sql": "SELECT MOD(b.intCol, 2), sum(a.intCol) AS colAlias FROM {tbl1} a INNER JOIN {tbl2} b ON a.strCol = b.strCol1 GROUP BY MOD(b.intCol, 2)" },


Shall we also ensure that logically equivalent SQL queries return same results ?

For example INNER JOIN query and a corresponding subquery with IN clause should return same results.

SELECT p.product_id, p.product_name FROM products p INNER JOIN categories c ON p.category_id = c.category_id WHERE c.category_id > 25

should return the same results as

SELECT p.product_id, p.product_name FROM products p WHERE p.category_id IN ( SELECT c.category_id FROM categories c WHERE c.category_id > 25 )

both are tested against a reference database. so yeah indirectly this is implied.

pinot-query-runtime/src/test/resources/queries/TableExpressions.json

siddharthteotia · 2022-11-17T17:20:22Z

pinot-query-runtime/src/test/resources/queries/TableExpressions.json

+      {
+        "ignored": true,
+        "comments": "Relation Decorrelator not supported",
+        "sql": "SELECT * FROM {tbl} AS a WHERE a.strCol IN (SELECT b.strCol FROM {tbl} AS b WHERE b.intCol = a.intCol + 1)"


For subquery in WHERE with IN, can we also add NOT IN ?

For IN clause subquery, let's also add test that the exact same query that uses ANY (supported by postgres) instead of IN returns identical results.

Same goes for SOME I guess

any and some will be tested in the function test instead of the table expression test

added. none are supported but added with ignore flag

siddharthteotia · 2022-11-17T17:35:44Z

pinot-query-runtime/src/test/resources/queries/TableExpressions.json

+      {
+        "ignored": true,
+        "comments": "EXISTS not supported",
+        "sql": "SELECT * FROM {tbl} AS a WHERE EXISTS (SELECT strCol FROM {tbl} AS b WHERE b.intCol = a.intCol + 1)"


Do the same for NOT EXISTS ?

siddharthteotia · 2022-11-17T17:45:01Z

pinot-query-runtime/src/test/resources/queries/TableExpressions.json

+      { "sql": "SELECT MOD(b.intCol, 2), sum(a.intCol) AS colAlias FROM {tbl1} a INNER JOIN {tbl2} b ON a.strCol = b.strCol1 GROUP BY MOD(b.intCol, 2)" },
+      { "sql": "SELECT strCol, SUM(intCol) FROM {tbl1} GROUP BY strCol HAVING SUM(intCol) > 3" },
+      { "sql": "SELECT strCol, SUM(intCol) FROM {tbl1} GROUP BY strCol HAVING AVG(intCol) > 1 AND MIN(intCol) < 10" },
+      { "sql": "SELECT strCol, b.strCol2, (sum(a.intCol) * b.intCol) AS colAlias FROM {tbl1} a INNER JOIN {tbl2} b ON a.strCol = b.strCol1 GROUP BY strCol, b.strCol2, b.intCol HAVING avg(a.intCol) < 100" }


Let's also add tests for subquery in the SELECT clause as a scalar operand ?

Something like ....

SELECT empID, lastName, firstName, ( CASE WHEN deptID = ( SELECT deptID FROM departments WHERE ...... ) THEN 'blah1' ELSE 'blah2' END) FROM emp

61yao · 2022-11-18T00:50:33Z

pinot-query-runtime/src/test/resources/queries/TableExpressions.json

+      }
+    ]
+  }
+}


Can we add select foo from (select 1) as foo?

i am not sure this is a valid query in most other SQL systems. did you meant

select foo.* from (select 1) as foo

foo is a table not a column name in this context, and postgres returns a table type which is not supported in Pinot

siddharthteotia · 2022-11-21T08:13:02Z

Thanks for addressing comment. LGTM

walterddr force-pushed the pr_add_table_tests branch from 948c8b6 to ef54003 Compare November 16, 2022 22:10

add table expression tests

f24ae62

walterddr force-pushed the pr_add_table_tests branch from ef54003 to f24ae62 Compare November 16, 2022 22:11

agavra approved these changes Nov 16, 2022

View reviewed changes

walterddr mentioned this pull request Nov 17, 2022

[multistage][test] add from expression queries #9782

Merged

adding more tests per comments

cd42e43

siddharthteotia reviewed Nov 17, 2022

View reviewed changes

pinot-query-runtime/src/test/resources/queries/TableExpressions.json Show resolved Hide resolved

siddharthteotia reviewed Nov 17, 2022

View reviewed changes

pinot-query-runtime/src/test/resources/queries/TableExpressions.json Show resolved Hide resolved

siddharthteotia reviewed Nov 17, 2022

View reviewed changes

pinot-query-runtime/src/test/resources/queries/TableExpressions.json Show resolved Hide resolved

siddharthteotia reviewed Nov 17, 2022

View reviewed changes

walterddr added the multi-stage Related to the multi-stage query engine label Nov 17, 2022

add more tests

73ed469

walterddr force-pushed the pr_add_table_tests branch from 3be0b46 to 73ed469 Compare November 17, 2022 18:41

61yao reviewed Nov 18, 2022

View reviewed changes

61yao approved these changes Nov 18, 2022

View reviewed changes

siddharthteotia approved these changes Nov 21, 2022

View reviewed changes

Merge branch 'master' into pr_add_table_tests

36cab68

walterddr merged commit 375b7d0 into apache:master Nov 28, 2022

walterddr deleted the pr_add_table_tests branch December 6, 2023 16:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[multistage][test] add table expression tests #9817

[multistage][test] add table expression tests #9817

walterddr commented Nov 16, 2022

agavra left a comment

agavra Nov 16, 2022

walterddr Nov 17, 2022

codecov-commenter commented Nov 16, 2022 •

edited

Loading

siddharthteotia Nov 17, 2022

walterddr Nov 17, 2022

siddharthteotia Nov 17, 2022 •

edited

Loading

walterddr Nov 17, 2022

siddharthteotia Nov 17, 2022 •

edited

Loading

walterddr Nov 17, 2022

siddharthteotia Nov 17, 2022

siddharthteotia Nov 17, 2022

siddharthteotia Nov 17, 2022

walterddr Nov 17, 2022

walterddr Nov 17, 2022

siddharthteotia Nov 17, 2022

walterddr Nov 17, 2022

siddharthteotia Nov 17, 2022

61yao Nov 18, 2022

walterddr Nov 18, 2022

siddharthteotia commented Nov 21, 2022

[multistage][test] add table expression tests #9817

[multistage][test] add table expression tests #9817

Conversation

walterddr commented Nov 16, 2022

agavra left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented Nov 16, 2022 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

siddharthteotia Nov 17, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

siddharthteotia Nov 17, 2022 • edited Loading

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

Choose a reason for hiding this comment

siddharthteotia commented Nov 21, 2022

codecov-commenter commented Nov 16, 2022 •

edited

Loading

siddharthteotia Nov 17, 2022 •

edited

Loading

siddharthteotia Nov 17, 2022 •

edited

Loading