You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "Jackie-Jiang (via GitHub)" <gi...@apache.org> on 2023/03/14 22:15:28 UTC
[GitHub] [pinot] Jackie-Jiang opened a new pull request, #10421: Add JOIN support to PinotQuery
Jackie-Jiang opened a new pull request, #10421:
URL: https://github.com/apache/pinot/pull/10421
Model `JOIN` as one type of the `DataSource`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #10421: Add JOIN support to PinotQuery
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #10421:
URL: https://github.com/apache/pinot/pull/10421#discussion_r1137829793
##########
pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java:
##########
@@ -492,6 +489,62 @@ public static PinotQuery compileSqlNodeToPinotQuery(SqlNode sqlNode) {
return pinotQuery;
}
+ private static DataSource compileToDataSource(SqlNode sqlNode) {
+ DataSource dataSource = new DataSource();
+ switch (sqlNode.getKind()) {
+ case IDENTIFIER:
+ dataSource.setTableName(sqlNode.toString());
+ break;
+ case AS:
+ List<SqlNode> operandList = ((SqlBasicCall) sqlNode).getOperandList();
+ dataSource.setSubquery(compileSqlNodeToPinotQuery(operandList.get(0)));
+ dataSource.setTableName(operandList.get(1).toString());
+ break;
+ case SELECT:
+ case ORDER_BY:
+ dataSource.setSubquery(compileSqlNodeToPinotQuery(sqlNode));
+ break;
+ case JOIN:
+ dataSource.setJoin(compileToJoin((SqlJoin) sqlNode));
+ break;
+ default:
+ throw new IllegalStateException("Unsupported SQL node kind as DataSource: " + sqlNode.getKind());
+ }
+ return dataSource;
+ }
+
+ private static Join compileToJoin(SqlJoin sqlJoin) {
+ Join join = new Join();
+ switch (sqlJoin.getJoinType()) {
+ case INNER:
+ join.setType(JoinType.INNER);
+ break;
+ case LEFT:
+ join.setType(JoinType.LEFT);
+ break;
+ case RIGHT:
+ join.setType(JoinType.RIGHT);
+ break;
+ case FULL:
+ join.setType(JoinType.FULL);
+ break;
+ default:
+ throw new IllegalStateException("Unsupported join type: " + sqlJoin.getJoinType());
+ }
+ join.setLeft(compileToDataSource(sqlJoin.getLeft()));
+ join.setRight(compileToDataSource(sqlJoin.getRight()));
+ switch (sqlJoin.getConditionType()) {
+ case ON:
+ join.setCondition(toExpression(sqlJoin.getCondition()));
Review Comment:
That is a very good optimization. Parser shouldn't be responsible for such optimization, but we should add an optimizer for this
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #10421: Add JOIN support to PinotQuery
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #10421:
URL: https://github.com/apache/pinot/pull/10421#discussion_r1137829047
##########
pinot-common/src/test/java/org/apache/pinot/sql/parsers/CalciteSqlCompilerTest.java:
##########
@@ -3054,15 +3067,88 @@ public void testExtractTableNamesFromNode() {
Assert.assertEquals(tableNames.get(3), "tbl4");
}
- private static SqlNodeAndOptions testSqlWithCustomSqlParser(String sqlString) {
- try (StringReader inStream = new StringReader(sqlString)) {
- SqlParserImpl sqlParser = CalciteSqlParser.newSqlParser(inStream);
- SqlNodeList sqlNodeList = sqlParser.SqlStmtsEof();
- // Extract OPTION statements from sql.
- return CalciteSqlParser.extractSqlNodeAndOptions(sqlString, sqlNodeList);
- } catch (Exception e) {
- Assert.fail("test custom sql parser failed", e);
- return null;
- }
+ @Test
+ public void testJoin() {
+ String query = "SELECT T1.a, T2.b FROM T1 JOIN T2";
+ PinotQuery pinotQuery = CalciteSqlParser.compileToPinotQuery(query);
+ DataSource dataSource = pinotQuery.getDataSource();
+ Assert.assertNull(dataSource.getTableName());
+ Assert.assertNull(dataSource.getSubquery());
+ Assert.assertNotNull(dataSource.getJoin());
+ Join join = dataSource.getJoin();
+ Assert.assertEquals(join.getType(), JoinType.INNER);
+ Assert.assertEquals(join.getLeft().getTableName(), "T1");
+ Assert.assertEquals(join.getRight().getTableName(), "T2");
+ Assert.assertNull(join.getCondition());
+
+ query = "SELECT T1.a, T2.b FROM T1 JOIN T2 ON T1.key = T2.key";
+ pinotQuery = CalciteSqlParser.compileToPinotQuery(query);
+ dataSource = pinotQuery.getDataSource();
+ Assert.assertNull(dataSource.getTableName());
+ Assert.assertNull(dataSource.getSubquery());
+ Assert.assertNotNull(dataSource.getJoin());
+ join = dataSource.getJoin();
+ Assert.assertEquals(join.getType(), JoinType.INNER);
+ Assert.assertEquals(join.getLeft().getTableName(), "T1");
+ Assert.assertEquals(join.getRight().getTableName(), "T2");
+ Assert.assertEquals(join.getCondition(), CalciteSqlParser.compileToExpression("T1.key = T2.key"));
+
+ query = "SELECT T1.a, T2.b FROM T1 FULL JOIN T2 ON T1.key = T2.key";
+ pinotQuery = CalciteSqlParser.compileToPinotQuery(query);
+ dataSource = pinotQuery.getDataSource();
+ Assert.assertNull(dataSource.getTableName());
+ Assert.assertNull(dataSource.getSubquery());
+ Assert.assertNotNull(dataSource.getJoin());
+ join = dataSource.getJoin();
+ Assert.assertEquals(join.getType(), JoinType.FULL);
+ Assert.assertEquals(join.getLeft().getTableName(), "T1");
+ Assert.assertEquals(join.getRight().getTableName(), "T2");
+ Assert.assertEquals(join.getCondition(), CalciteSqlParser.compileToExpression("T1.key = T2.key"));
+
+ query = "SELECT T1.a, T2.b FROM T1 LEFT JOIN T2 ON T1.a > T2.b";
+ pinotQuery = CalciteSqlParser.compileToPinotQuery(query);
+ dataSource = pinotQuery.getDataSource();
+ Assert.assertNull(dataSource.getTableName());
+ Assert.assertNull(dataSource.getSubquery());
+ Assert.assertNotNull(dataSource.getJoin());
+ join = dataSource.getJoin();
+ Assert.assertEquals(join.getType(), JoinType.LEFT);
+ Assert.assertEquals(join.getLeft().getTableName(), "T1");
+ Assert.assertEquals(join.getRight().getTableName(), "T2");
+ Assert.assertEquals(join.getCondition(), CalciteSqlParser.compileToExpression("T1.a > T2.b"));
+
+ query =
+ "SELECT T1.a, T2.b FROM T1 RIGHT JOIN (SELECT a, COUNT(*) AS b FROM T3 GROUP BY a) AS T2 ON T1.key = T2.key";
+ pinotQuery = CalciteSqlParser.compileToPinotQuery(query);
+ dataSource = pinotQuery.getDataSource();
+ Assert.assertNull(dataSource.getTableName());
+ Assert.assertNull(dataSource.getSubquery());
+ Assert.assertNotNull(dataSource.getJoin());
+ join = dataSource.getJoin();
+ Assert.assertEquals(join.getType(), JoinType.RIGHT);
+ Assert.assertEquals(join.getLeft().getTableName(), "T1");
+ DataSource right = join.getRight();
+ Assert.assertEquals(right.getTableName(), "T2");
+ PinotQuery rightSubquery = right.getSubquery();
+ Assert.assertEquals(rightSubquery,
+ CalciteSqlParser.compileToPinotQuery("SELECT a, COUNT(*) AS b FROM T3 GROUP BY a"));
+ Assert.assertEquals(join.getCondition(), CalciteSqlParser.compileToExpression("T1.key = T2.key"));
+
+ query =
+ "SELECT T1.a, T2.b FROM T1 JOIN (SELECT a, COUNT(*) AS b FROM T3 JOIN T4 GROUP BY a) AS T2 ON T1.key = T2.key";
Review Comment:
Good catch. Clearly this check is out of the parser's power lol
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] siddharthteotia commented on a diff in pull request #10421: Add JOIN support to PinotQuery
Posted by "siddharthteotia (via GitHub)" <gi...@apache.org>.
siddharthteotia commented on code in PR #10421:
URL: https://github.com/apache/pinot/pull/10421#discussion_r1140967721
##########
pinot-common/src/test/java/org/apache/pinot/sql/parsers/CalciteSqlCompilerTest.java:
##########
@@ -3054,15 +3067,88 @@ public void testExtractTableNamesFromNode() {
Assert.assertEquals(tableNames.get(3), "tbl4");
}
- private static SqlNodeAndOptions testSqlWithCustomSqlParser(String sqlString) {
- try (StringReader inStream = new StringReader(sqlString)) {
- SqlParserImpl sqlParser = CalciteSqlParser.newSqlParser(inStream);
- SqlNodeList sqlNodeList = sqlParser.SqlStmtsEof();
- // Extract OPTION statements from sql.
- return CalciteSqlParser.extractSqlNodeAndOptions(sqlString, sqlNodeList);
- } catch (Exception e) {
- Assert.fail("test custom sql parser failed", e);
- return null;
- }
+ @Test
+ public void testJoin() {
+ String query = "SELECT T1.a, T2.b FROM T1 JOIN T2";
+ PinotQuery pinotQuery = CalciteSqlParser.compileToPinotQuery(query);
+ DataSource dataSource = pinotQuery.getDataSource();
+ Assert.assertNull(dataSource.getTableName());
+ Assert.assertNull(dataSource.getSubquery());
+ Assert.assertNotNull(dataSource.getJoin());
+ Join join = dataSource.getJoin();
+ Assert.assertEquals(join.getType(), JoinType.INNER);
+ Assert.assertEquals(join.getLeft().getTableName(), "T1");
+ Assert.assertEquals(join.getRight().getTableName(), "T2");
+ Assert.assertNull(join.getCondition());
+
+ query = "SELECT T1.a, T2.b FROM T1 JOIN T2 ON T1.key = T2.key";
Review Comment:
May want to add a test that specifically uses `INNER JOIN` keyword in the SQL
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] walterddr commented on a diff in pull request #10421: Add JOIN support to PinotQuery
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on code in PR #10421:
URL: https://github.com/apache/pinot/pull/10421#discussion_r1137845840
##########
pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java:
##########
@@ -492,6 +489,62 @@ public static PinotQuery compileSqlNodeToPinotQuery(SqlNode sqlNode) {
return pinotQuery;
}
+ private static DataSource compileToDataSource(SqlNode sqlNode) {
+ DataSource dataSource = new DataSource();
+ switch (sqlNode.getKind()) {
+ case IDENTIFIER:
+ dataSource.setTableName(sqlNode.toString());
+ break;
+ case AS:
+ List<SqlNode> operandList = ((SqlBasicCall) sqlNode).getOperandList();
+ dataSource.setSubquery(compileSqlNodeToPinotQuery(operandList.get(0)));
+ dataSource.setTableName(operandList.get(1).toString());
+ break;
+ case SELECT:
+ case ORDER_BY:
+ dataSource.setSubquery(compileSqlNodeToPinotQuery(sqlNode));
+ break;
+ case JOIN:
+ dataSource.setJoin(compileToJoin((SqlJoin) sqlNode));
+ break;
+ default:
+ throw new IllegalStateException("Unsupported SQL node kind as DataSource: " + sqlNode.getKind());
+ }
+ return dataSource;
+ }
+
+ private static Join compileToJoin(SqlJoin sqlJoin) {
+ Join join = new Join();
+ switch (sqlJoin.getJoinType()) {
+ case INNER:
+ join.setType(JoinType.INNER);
+ break;
+ case LEFT:
+ join.setType(JoinType.LEFT);
+ break;
+ case RIGHT:
+ join.setType(JoinType.RIGHT);
+ break;
+ case FULL:
+ join.setType(JoinType.FULL);
+ break;
+ default:
+ throw new IllegalStateException("Unsupported join type: " + sqlJoin.getJoinType());
+ }
+ join.setLeft(compileToDataSource(sqlJoin.getLeft()));
+ join.setRight(compileToDataSource(sqlJoin.getRight()));
+ switch (sqlJoin.getConditionType()) {
+ case ON:
+ join.setCondition(toExpression(sqlJoin.getCondition()));
Review Comment:
you mean query rewriter? i think we don't need to deal with these level of complexity in parser
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang merged pull request #10421: Add JOIN support to PinotQuery
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang merged PR #10421:
URL: https://github.com/apache/pinot/pull/10421
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter commented on pull request #10421: Add JOIN support to PinotQuery
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #10421:
URL: https://github.com/apache/pinot/pull/10421#issuecomment-1469016708
## [Codecov](https://codecov.io/gh/apache/pinot/pull/10421?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#10421](https://codecov.io/gh/apache/pinot/pull/10421?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (bd7536e) into [master](https://codecov.io/gh/apache/pinot/commit/01ff18d47caeaee6e31b09b1b6486f756eff0db1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (01ff18d) will **decrease** coverage by `36.60%`.
> The diff coverage is `14.70%`.
```diff
@@ Coverage Diff @@
## master #10421 +/- ##
=============================================
- Coverage 60.94% 24.35% -36.60%
+ Complexity 5060 58 -5002
=============================================
Files 2028 2028
Lines 110627 110656 +29
Branches 16846 16848 +2
=============================================
- Hits 67427 26947 -40480
- Misses 38154 80940 +42786
+ Partials 5046 2769 -2277
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `24.35% <14.70%> (?)` | |
| unittests1 | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/10421?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...org/apache/pinot/sql/parsers/CalciteSqlParser.java](https://codecov.io/gh/apache/pinot/pull/10421?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcWwvcGFyc2Vycy9DYWxjaXRlU3FsUGFyc2VyLmphdmE=) | `63.46% <14.70%> (-20.98%)` | :arrow_down: |
... and [1391 files with indirect coverage changes](https://codecov.io/gh/apache/pinot/pull/10421/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] walterddr commented on a diff in pull request #10421: Add JOIN support to PinotQuery
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on code in PR #10421:
URL: https://github.com/apache/pinot/pull/10421#discussion_r1137592476
##########
pinot-common/src/test/java/org/apache/pinot/sql/parsers/CalciteSqlCompilerTest.java:
##########
@@ -3054,15 +3067,88 @@ public void testExtractTableNamesFromNode() {
Assert.assertEquals(tableNames.get(3), "tbl4");
}
- private static SqlNodeAndOptions testSqlWithCustomSqlParser(String sqlString) {
- try (StringReader inStream = new StringReader(sqlString)) {
- SqlParserImpl sqlParser = CalciteSqlParser.newSqlParser(inStream);
- SqlNodeList sqlNodeList = sqlParser.SqlStmtsEof();
- // Extract OPTION statements from sql.
- return CalciteSqlParser.extractSqlNodeAndOptions(sqlString, sqlNodeList);
- } catch (Exception e) {
- Assert.fail("test custom sql parser failed", e);
- return null;
- }
+ @Test
+ public void testJoin() {
+ String query = "SELECT T1.a, T2.b FROM T1 JOIN T2";
+ PinotQuery pinotQuery = CalciteSqlParser.compileToPinotQuery(query);
+ DataSource dataSource = pinotQuery.getDataSource();
+ Assert.assertNull(dataSource.getTableName());
+ Assert.assertNull(dataSource.getSubquery());
+ Assert.assertNotNull(dataSource.getJoin());
+ Join join = dataSource.getJoin();
+ Assert.assertEquals(join.getType(), JoinType.INNER);
+ Assert.assertEquals(join.getLeft().getTableName(), "T1");
+ Assert.assertEquals(join.getRight().getTableName(), "T2");
+ Assert.assertNull(join.getCondition());
+
+ query = "SELECT T1.a, T2.b FROM T1 JOIN T2 ON T1.key = T2.key";
+ pinotQuery = CalciteSqlParser.compileToPinotQuery(query);
+ dataSource = pinotQuery.getDataSource();
+ Assert.assertNull(dataSource.getTableName());
+ Assert.assertNull(dataSource.getSubquery());
+ Assert.assertNotNull(dataSource.getJoin());
+ join = dataSource.getJoin();
+ Assert.assertEquals(join.getType(), JoinType.INNER);
+ Assert.assertEquals(join.getLeft().getTableName(), "T1");
+ Assert.assertEquals(join.getRight().getTableName(), "T2");
+ Assert.assertEquals(join.getCondition(), CalciteSqlParser.compileToExpression("T1.key = T2.key"));
+
+ query = "SELECT T1.a, T2.b FROM T1 FULL JOIN T2 ON T1.key = T2.key";
+ pinotQuery = CalciteSqlParser.compileToPinotQuery(query);
+ dataSource = pinotQuery.getDataSource();
+ Assert.assertNull(dataSource.getTableName());
+ Assert.assertNull(dataSource.getSubquery());
+ Assert.assertNotNull(dataSource.getJoin());
+ join = dataSource.getJoin();
+ Assert.assertEquals(join.getType(), JoinType.FULL);
+ Assert.assertEquals(join.getLeft().getTableName(), "T1");
+ Assert.assertEquals(join.getRight().getTableName(), "T2");
+ Assert.assertEquals(join.getCondition(), CalciteSqlParser.compileToExpression("T1.key = T2.key"));
+
+ query = "SELECT T1.a, T2.b FROM T1 LEFT JOIN T2 ON T1.a > T2.b";
+ pinotQuery = CalciteSqlParser.compileToPinotQuery(query);
+ dataSource = pinotQuery.getDataSource();
+ Assert.assertNull(dataSource.getTableName());
+ Assert.assertNull(dataSource.getSubquery());
+ Assert.assertNotNull(dataSource.getJoin());
+ join = dataSource.getJoin();
+ Assert.assertEquals(join.getType(), JoinType.LEFT);
+ Assert.assertEquals(join.getLeft().getTableName(), "T1");
+ Assert.assertEquals(join.getRight().getTableName(), "T2");
+ Assert.assertEquals(join.getCondition(), CalciteSqlParser.compileToExpression("T1.a > T2.b"));
+
+ query =
+ "SELECT T1.a, T2.b FROM T1 RIGHT JOIN (SELECT a, COUNT(*) AS b FROM T3 GROUP BY a) AS T2 ON T1.key = T2.key";
+ pinotQuery = CalciteSqlParser.compileToPinotQuery(query);
+ dataSource = pinotQuery.getDataSource();
+ Assert.assertNull(dataSource.getTableName());
+ Assert.assertNull(dataSource.getSubquery());
+ Assert.assertNotNull(dataSource.getJoin());
+ join = dataSource.getJoin();
+ Assert.assertEquals(join.getType(), JoinType.RIGHT);
+ Assert.assertEquals(join.getLeft().getTableName(), "T1");
+ DataSource right = join.getRight();
+ Assert.assertEquals(right.getTableName(), "T2");
+ PinotQuery rightSubquery = right.getSubquery();
+ Assert.assertEquals(rightSubquery,
+ CalciteSqlParser.compileToPinotQuery("SELECT a, COUNT(*) AS b FROM T3 GROUP BY a"));
+ Assert.assertEquals(join.getCondition(), CalciteSqlParser.compileToExpression("T1.key = T2.key"));
+
+ query =
+ "SELECT T1.a, T2.b FROM T1 JOIN (SELECT a, COUNT(*) AS b FROM T3 JOIN T4 GROUP BY a) AS T2 ON T1.key = T2.key";
Review Comment:
this query is invalid right? the result of T2 doesn't have key column
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] 61yao commented on a diff in pull request #10421: Add JOIN support to PinotQuery
Posted by "61yao (via GitHub)" <gi...@apache.org>.
61yao commented on code in PR #10421:
URL: https://github.com/apache/pinot/pull/10421#discussion_r1137763923
##########
pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java:
##########
@@ -492,6 +489,62 @@ public static PinotQuery compileSqlNodeToPinotQuery(SqlNode sqlNode) {
return pinotQuery;
}
+ private static DataSource compileToDataSource(SqlNode sqlNode) {
+ DataSource dataSource = new DataSource();
+ switch (sqlNode.getKind()) {
+ case IDENTIFIER:
+ dataSource.setTableName(sqlNode.toString());
+ break;
+ case AS:
+ List<SqlNode> operandList = ((SqlBasicCall) sqlNode).getOperandList();
+ dataSource.setSubquery(compileSqlNodeToPinotQuery(operandList.get(0)));
+ dataSource.setTableName(operandList.get(1).toString());
+ break;
+ case SELECT:
+ case ORDER_BY:
+ dataSource.setSubquery(compileSqlNodeToPinotQuery(sqlNode));
+ break;
+ case JOIN:
+ dataSource.setJoin(compileToJoin((SqlJoin) sqlNode));
+ break;
+ default:
+ throw new IllegalStateException("Unsupported SQL node kind as DataSource: " + sqlNode.getKind());
+ }
+ return dataSource;
+ }
+
+ private static Join compileToJoin(SqlJoin sqlJoin) {
+ Join join = new Join();
+ switch (sqlJoin.getJoinType()) {
+ case INNER:
+ join.setType(JoinType.INNER);
+ break;
+ case LEFT:
+ join.setType(JoinType.LEFT);
+ break;
+ case RIGHT:
+ join.setType(JoinType.RIGHT);
+ break;
+ case FULL:
+ join.setType(JoinType.FULL);
+ break;
+ default:
+ throw new IllegalStateException("Unsupported join type: " + sqlJoin.getJoinType());
+ }
+ join.setLeft(compileToDataSource(sqlJoin.getLeft()));
+ join.setRight(compileToDataSource(sqlJoin.getRight()));
+ switch (sqlJoin.getConditionType()) {
+ case ON:
+ join.setCondition(toExpression(sqlJoin.getCondition()));
Review Comment:
It would be better if we can push left and right condition to left and right data source. and only keep the condition that need from both left and right side in join condition. but it is a little bit hard to doo using SqlNode. :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] xiangfu0 commented on pull request #10421: Add JOIN support to PinotQuery
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on PR #10421:
URL: https://github.com/apache/pinot/pull/10421#issuecomment-1483696373
how do you think we can model a lateral unnest?
E.g.
SELECT o.order_id, o.customer_id, p.id AS product_id, p.name, p.price
FROM orders o,
LATERAL unnest(o.product_ids) AS product_id
JOIN products p ON p.id = product_id;
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org