You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "tuichenchuxin (via GitHub)" <gi...@apache.org> on 2025/03/07 02:08:14 UTC
[PR] [CALCITE-2115] Fix MongoAdapterTest [calcite]
tuichenchuxin opened a new pull request, #4228:
URL: https://github.com/apache/calcite/pull/4228
- Since the implementation of group distinct is implemented through mongo API, group, etc. are deleted in the statement.
- sales_fact_1997， sales_fact_1998 has been removed from the database. So can't be found in schema. I think these test for sales_fact_1997, sales_fact_1998 can be removed.
https://issues.apache.org/jira/browse/CALCITE-2115
--
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@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2115] Remove not generated mongo doc, and popular sales_fact_1997/1998 data [calcite]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #4228: [CALCITE-2115] Remove not generated mongo doc, and popular sales_fact_1997/1998 data
URL: https://github.com/apache/calcite/pull/4228
--
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@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2115] Remove not generated mongo doc, and remove useless test [calcite]
Posted by "tuichenchuxin (via GitHub)" <gi...@apache.org>.
tuichenchuxin commented on PR #4228:
URL: https://github.com/apache/calcite/pull/4228#issuecomment-2705854527
> question is, what version of MongoDB is Calcite based on? Do we need to explain it?
Thank you for review.
I think we need to explain what version we can support. for example 4.2 + can be supported.
I think the latest version should be compatible with the old mongodb version. So we just need to confirm the minimum supported version.
--
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@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2115] Remove not generated mongo doc, and remove useless test [calcite]
Posted by "tuichenchuxin (via GitHub)" <gi...@apache.org>.
tuichenchuxin commented on code in PR #4228:
URL: https://github.com/apache/calcite/pull/4228#discussion_r1986636427
##########
mongodb/src/test/java/org/apache/calcite/adapter/mongodb/MongoAdapterTest.java:
##########
@@ -284,37 +284,6 @@ private CalciteAssert.AssertThat assertModel(URL url) {
"CITY=LAWTON; LONGITUDE=null; LATITUDE=null; POP=45542; STATE=OK; ID=73505");
}
- @Disabled("broken; [CALCITE-2115] is logged to fix it")
Review Comment:
> I don't understand why we directly deleted the test cases that couldn't run. A better approach should be to make them run as we want.
OK, thank you for your suggestions, so I will add some data so that these test cases can run
--
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@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2115] Remove not generated mongo doc, and remove useless test [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on PR #4228:
URL: https://github.com/apache/calcite/pull/4228#issuecomment-2705781700
A very important question is, what version of MongoDB is Calcite based on? Do we need to explain it?
--
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@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2115] Remove not generated mongo doc, and remove useless test [calcite]
Posted by "NobiGo (via GitHub)" <gi...@apache.org>.
NobiGo commented on code in PR #4228:
URL: https://github.com/apache/calcite/pull/4228#discussion_r1984902483
##########
mongodb/src/test/java/org/apache/calcite/adapter/mongodb/MongoAdapterTest.java:
##########
@@ -284,37 +284,6 @@ private CalciteAssert.AssertThat assertModel(URL url) {
"CITY=LAWTON; LONGITUDE=null; LATITUDE=null; POP=45542; STATE=OK; ID=73505");
}
- @Disabled("broken; [CALCITE-2115] is logged to fix it")
Review Comment:
I don't understand why we directly deleted the test cases that couldn't run. A better approach should be to make them run as we want.
--
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@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2115] Remove not generated mongo doc, and popular sales_fact_1997/1998 data [calcite]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #4228:
URL: https://github.com/apache/calcite/pull/4228#issuecomment-4437393578
This pull request has been closed due to lack of activity. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.
--
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@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2115] Remove not generated mongo doc, and popular sales_fact_1997/1998 data [calcite]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #4228:
URL: https://github.com/apache/calcite/pull/4228#issuecomment-2795759831
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 90 days if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@calcite.apache.org list. Thank you for your contributions.
--
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@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2115] Remove not generated mongo doc, and popular sales_fact_1997/1998 data [calcite]
Posted by "tuichenchuxin (via GitHub)" <gi...@apache.org>.
tuichenchuxin commented on code in PR #4228:
URL: https://github.com/apache/calcite/pull/4228#discussion_r1988266851
##########
mongodb/src/test/java/org/apache/calcite/adapter/mongodb/MongoAdapterTest.java:
##########
@@ -284,37 +284,6 @@ private CalciteAssert.AssertThat assertModel(URL url) {
"CITY=LAWTON; LONGITUDE=null; LATITUDE=null; POP=45542; STATE=OK; ID=73505");
}
- @Disabled("broken; [CALCITE-2115] is logged to fix it")
Review Comment:
@NobiGo I found sales_for_1997 is in foodmart-data-json-0.4.jar, so I popular it, then test can run correctly.
--
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@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2115] Remove not generated mongo doc, and popular sales_fact_1997/1998 data [calcite]
Posted by "tuichenchuxin (via GitHub)" <gi...@apache.org>.
tuichenchuxin commented on code in PR #4228:
URL: https://github.com/apache/calcite/pull/4228#discussion_r1988310846
##########
mongodb/src/test/java/org/apache/calcite/adapter/mongodb/MongoAdapterTest.java:
##########
@@ -605,12 +607,7 @@ private CalciteAssert.AssertThat assertModel(URL url) {
+ " ]\n"
+ " }\n"
+ "}",
- "{$project: {CITY: '$city', STATE: '$state'}}",
- "{$group: {_id: {CITY: '$CITY', STATE: '$STATE'}}}",
- "{$project: {_id: 0, CITY: '$_id.CITY', STATE: '$_id.STATE'}}",
- "{$group: {_id: '$STATE', CDC: {$sum: {$cond: [ {$eq: ['CITY', null]}, 0, 1]}}}}",
- "{$project: {STATE: '$_id', CDC: '$CDC'}}",
- "{$sort: {STATE: 1}}"));
+ "{$project: {STATE: '$state', CITY: '$city'}}"));
Review Comment:
I found the implementation of group and count distinct is not implemented by mongo statements, but through code and API
https://github.com/apache/calcite/blob/618e601b136e92db933523f77dd7af3c1dfe2779/mongodb/src/main/java/org/apache/calcite/adapter/mongodb/MongoToEnumerableConverter.java#L113-L116
https://github.com/apache/calcite/blob/618e601b136e92db933523f77dd7af3c1dfe2779/mongodb/src/main/java/org/apache/calcite/adapter/mongodb/MongoTable.java#L131-L153
--
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@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2115] Remove not generated mongo doc, and popular sales_fact_1997/1998 data [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4228:
URL: https://github.com/apache/calcite/pull/4228#issuecomment-2716236155
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4228) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4228&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4228&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4228&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4228&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4228&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4228)
--
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@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2115] Remove not generated mongo doc, and popular sales_fact_1997/1998 data [calcite]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #4228:
URL: https://github.com/apache/calcite/pull/4228#issuecomment-3445640516
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 90 days if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@calcite.apache.org list. Thank you for your contributions.
--
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@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2115] Remove not generated mongo doc, and popular sales_fact_1997/1998 data [calcite]
Posted by "NobiGo (via GitHub)" <gi...@apache.org>.
NobiGo commented on code in PR #4228:
URL: https://github.com/apache/calcite/pull/4228#discussion_r1988281117
##########
mongodb/src/test/java/org/apache/calcite/adapter/mongodb/MongoAdapterTest.java:
##########
@@ -605,12 +607,7 @@ private CalciteAssert.AssertThat assertModel(URL url) {
+ " ]\n"
+ " }\n"
+ "}",
- "{$project: {CITY: '$city', STATE: '$state'}}",
- "{$group: {_id: {CITY: '$CITY', STATE: '$STATE'}}}",
- "{$project: {_id: 0, CITY: '$_id.CITY', STATE: '$_id.STATE'}}",
- "{$group: {_id: '$STATE', CDC: {$sum: {$cond: [ {$eq: ['CITY', null]}, 0, 1]}}}}",
- "{$project: {STATE: '$_id', CDC: '$CDC'}}",
- "{$sort: {STATE: 1}}"));
+ "{$project: {STATE: '$state', CITY: '$city'}}"));
Review Comment:
I'm not very familiar with the mongo adapter. However, judging from the plan, the script here seems to have problems. The previous test cases looked correct from the perspective of SQL semantics. But now this plan has lost the `group by` and `order by` clauses. I guess we can check whether the related operations have been completed through Enumerable by adding explainContains.
--
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@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2115] Remove not generated mongo doc, and popular sales_fact_1997/1998 data [calcite]
Posted by "tuichenchuxin (via GitHub)" <gi...@apache.org>.
tuichenchuxin commented on code in PR #4228:
URL: https://github.com/apache/calcite/pull/4228#discussion_r1990416993
##########
mongodb/src/test/java/org/apache/calcite/adapter/mongodb/MongoAdapterTest.java:
##########
@@ -605,12 +607,7 @@ private CalciteAssert.AssertThat assertModel(URL url) {
+ " ]\n"
+ " }\n"
+ "}",
- "{$project: {CITY: '$city', STATE: '$state'}}",
- "{$group: {_id: {CITY: '$CITY', STATE: '$STATE'}}}",
- "{$project: {_id: 0, CITY: '$_id.CITY', STATE: '$_id.STATE'}}",
- "{$group: {_id: '$STATE', CDC: {$sum: {$cond: [ {$eq: ['CITY', null]}, 0, 1]}}}}",
- "{$project: {STATE: '$_id', CDC: '$CDC'}}",
- "{$sort: {STATE: 1}}"));
+ "{$project: {STATE: '$state', CITY: '$city'}}"));
Review Comment:
@NobiGo Thank you for your suggestion, I have added the explainContains, and the plan indicates the related operations have been completed.
--
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@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2115] Remove not generated mongo doc, and popular sales_fact_1997/1998 data [calcite]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #4228:
URL: https://github.com/apache/calcite/pull/4228#issuecomment-3888596729
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 90 days if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@calcite.apache.org list. Thank you for your contributions.
--
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@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org