You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by GitBox <gi...@apache.org> on 2021/05/26 01:36:19 UTC
[GitHub] [iotdb] yanhongwangg opened a new pull request #2460: fix show devices
yanhongwangg opened a new pull request #2460:
URL: https://github.com/apache/iotdb/pull/2460
before，when i create a timeseries with three nodes an then execute show devices
after，same operation
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] sonarcloud[bot] commented on pull request #2460: fix show devices
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #2460:
URL: https://github.com/apache/iotdb/pull/2460#issuecomment-757123853
Kudos, SonarCloud Quality Gate passed!
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2460&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2460&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2460&resolved=false&types=BUG)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2460&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2460&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2460&resolved=false&types=VULNERABILITY)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=2460&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=2460&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=2460&resolved=false&types=SECURITY_HOTSPOT)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2460&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2460&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2460&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo.png' alt='No Coverage information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=2460) No Coverage information
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/NoDuplicationInfo.png' alt='No Duplication information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=2460&metric=duplicated_lines_density&view=list) No Duplication information
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] yanhongwangg commented on pull request #2460: fix show devices
Posted by GitBox <gi...@apache.org>.
yanhongwangg commented on pull request #2460:
URL: https://github.com/apache/iotdb/pull/2460#issuecomment-768268506
> I do not think you get my point. I was saying adding `.*` to the paths is not necessarily a bug, as it may be needed in data queries. You should either convince me by showing that this change does not affect related data queries or allow `show device` and other queries to ignore the last `.*`.
Thanks for your guidence, I got it, I'll make changes in show devices to ignore the last one*
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] sonarcloud[bot] commented on pull request #2460: fix show devices
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #2460:
URL: https://github.com/apache/iotdb/pull/2460#issuecomment-757123853
Kudos, SonarCloud Quality Gate passed!
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2460&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2460&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2460&resolved=false&types=BUG)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2460&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2460&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2460&resolved=false&types=VULNERABILITY)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=2460&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=2460&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=2460&resolved=false&types=SECURITY_HOTSPOT)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2460&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2460&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2460&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo.png' alt='No Coverage information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=2460) No Coverage information
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/NoDuplicationInfo.png' alt='No Duplication information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=2460&metric=duplicated_lines_density&view=list) No Duplication information
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] yanhongwangg closed pull request #2460: fix show devices
Posted by GitBox <gi...@apache.org>.
yanhongwangg closed pull request #2460:
URL: https://github.com/apache/iotdb/pull/2460
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] yanhongwangg commented on pull request #2460: fix show devices
Posted by GitBox <gi...@apache.org>.
yanhongwangg commented on pull request #2460:
URL: https://github.com/apache/iotdb/pull/2460#issuecomment-766586220
> How this may affect data queries need to be further investigated. For example, if this change results in `root.group1.*` being replaced by `root.group1` in a data query, the result will be very different.
> I suggest a modification in `show devices` rather than here.
Thanks for your advice,but this bug occurs before getting devices according to the path, so there is no way to change it when getting devices，meanwhile, I looked at it and "*" is not used.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] jt2594838 edited a comment on pull request #2460: fix show devices
Posted by GitBox <gi...@apache.org>.
jt2594838 edited a comment on pull request #2460:
URL: https://github.com/apache/iotdb/pull/2460#issuecomment-767958599
I do not think you get my point. I was saying adding `.*` to the paths is not necessarily a bug, as it may be needed in data queries. You should either convince me by showing that this change does not affect related data queries or allow `show device` and other queries to ignore the last `.*`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] yanhongwangg removed a comment on pull request #2460: fix show devices
Posted by GitBox <gi...@apache.org>.
yanhongwangg removed a comment on pull request #2460:
URL: https://github.com/apache/iotdb/pull/2460#issuecomment-768871649
> ignore the last `.*`.
Maybe i can fix it in show devices by ignoring the last .*., but there is another question
before ignoring the last .*., when i execute show devices root.a.b.* ,the result is root.a.b.c,
after ignoring the last .*., execute the same sql ,it will match root.a.b and root.a.b.c,
result is inconsistent.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] jt2594838 commented on pull request #2460: fix show devices
Posted by GitBox <gi...@apache.org>.
jt2594838 commented on pull request #2460:
URL: https://github.com/apache/iotdb/pull/2460#issuecomment-765063772
How this may affect data queries need to be further investigated. For example, if this change results in `root.group1.*` being replaced by `root.group1` in a data query, the result will be very different.
I suggest a modification in `show devices` rather than here.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] HTHou commented on pull request #2460: fix show devices
Posted by GitBox <gi...@apache.org>.
HTHou commented on pull request #2460:
URL: https://github.com/apache/iotdb/pull/2460#issuecomment-757677369
Good job, Yanhong!
Consider add an UT or 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] jt2594838 removed a comment on pull request #2460: fix show devices
Posted by GitBox <gi...@apache.org>.
jt2594838 removed a comment on pull request #2460:
URL: https://github.com/apache/iotdb/pull/2460#issuecomment-769626779
> > I do not think you get my point. I was saying adding `.*` to the paths is not necessarily a bug, as it may be needed in data queries. You should either convince me by showing that this change does not affect related data queries or allow `show device` and other queries to ignore the last `.*`.
>
> Maybe i can fix it in show devices by ignoring the last ._., but there is another question before ignoring the last ._., when i execute show devices root.a.b.* ,the result is root.a.b.c,
> after ignoring the last .*., execute the same sql ,it will match root.a.b and root.a.b.c,
> result is inconsistent.
How is it possible? Only one of `root.a.b` and `root.a.b.c` can be a device if you fix it 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] yanhongwangg closed pull request #2460: fix show devices
Posted by GitBox <gi...@apache.org>.
yanhongwangg closed pull request #2460:
URL: https://github.com/apache/iotdb/pull/2460
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] jixuan1989 commented on pull request #2460: fix show devices
Posted by GitBox <gi...@apache.org>.
jixuan1989 commented on pull request #2460:
URL: https://github.com/apache/iotdb/pull/2460#issuecomment-764701656
ping
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] jt2594838 commented on pull request #2460: fix show devices
Posted by GitBox <gi...@apache.org>.
jt2594838 commented on pull request #2460:
URL: https://github.com/apache/iotdb/pull/2460#issuecomment-769626779
> > I do not think you get my point. I was saying adding `.*` to the paths is not necessarily a bug, as it may be needed in data queries. You should either convince me by showing that this change does not affect related data queries or allow `show device` and other queries to ignore the last `.*`.
>
> Maybe i can fix it in show devices by ignoring the last ._., but there is another question before ignoring the last ._., when i execute show devices root.a.b.* ,the result is root.a.b.c,
> after ignoring the last .*., execute the same sql ,it will match root.a.b and root.a.b.c,
> result is inconsistent.
How is it possible? Only one of `root.a.b` and `root.a.b.c` can be a device if you fix it 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] jt2594838 commented on pull request #2460: fix show devices
Posted by GitBox <gi...@apache.org>.
jt2594838 commented on pull request #2460:
URL: https://github.com/apache/iotdb/pull/2460#issuecomment-765063772
How this may affect data queries need to be further investigated. For example, if this change results in `root.group1.*` being replaced by `root.group1` in a data query, the result will be very different.
I suggest a modification in `show devices` rather than here.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] jixuan1989 commented on pull request #2460: fix show devices
Posted by GitBox <gi...@apache.org>.
jixuan1989 commented on pull request #2460:
URL: https://github.com/apache/iotdb/pull/2460#issuecomment-764701656
ping
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] jt2594838 commented on pull request #2460: fix show devices
Posted by GitBox <gi...@apache.org>.
jt2594838 commented on pull request #2460:
URL: https://github.com/apache/iotdb/pull/2460#issuecomment-767963894
For example, assuming we have 3 SGs: root.sg1, root.sg2, and root.sg3.
And now we execute `SELECT * FROM root`, the removed code will convert this SQL into "SELECT root.sg1.*, root.sg2.*, root.sg3.*", while after the removal, the result may be "SELECT root.sg1, root.sg2, root.s3", which is apparently a different one.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] yanhongwangg commented on pull request #2460: fix show devices
Posted by GitBox <gi...@apache.org>.
yanhongwangg commented on pull request #2460:
URL: https://github.com/apache/iotdb/pull/2460#issuecomment-768871939
> I do not think you get my point. I was saying adding `.*` to the paths is not necessarily a bug, as it may be needed in data queries. You should either convince me by showing that this change does not affect related data queries or allow `show device` and other queries to ignore the last `.*`.
Maybe i can fix it in show devices by ignoring the last .*., but there is another question
before ignoring the last .*., when i execute show devices root.a.b.* ,the result is root.a.b.c,
after ignoring the last .*., execute the same sql ,it will match root.a.b and root.a.b.c,
result is inconsistent.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] yanhongwangg removed a comment on pull request #2460: fix show devices
Posted by GitBox <gi...@apache.org>.
yanhongwangg removed a comment on pull request #2460:
URL: https://github.com/apache/iotdb/pull/2460#issuecomment-768268506
> I do not think you get my point. I was saying adding `.*` to the paths is not necessarily a bug, as it may be needed in data queries. You should either convince me by showing that this change does not affect related data queries or allow `show device` and other queries to ignore the last `.*`.
Thanks for your guidence, I got it, I'll make changes in show devices to ignore the last one*
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] jt2594838 commented on pull request #2460: fix show devices
Posted by GitBox <gi...@apache.org>.
jt2594838 commented on pull request #2460:
URL: https://github.com/apache/iotdb/pull/2460#issuecomment-767958599
I do not think you get my point. I was saying adding ".*" to the paths is not necessarily a bug, as it may be needed in data queries. You should either convince me by showing that this change does not affect related data queries or allow `show device` and other queries to ignore the last ".*".
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] yanhongwangg commented on pull request #2460: fix show devices
Posted by GitBox <gi...@apache.org>.
yanhongwangg commented on pull request #2460:
URL: https://github.com/apache/iotdb/pull/2460#issuecomment-766586220
> How this may affect data queries need to be further investigated. For example, if this change results in `root.group1.*` being replaced by `root.group1` in a data query, the result will be very different.
> I suggest a modification in `show devices` rather than here.
Thanks for your advice,but this bug occurs before getting devices according to the path, so there is no way to change it when getting devices，meanwhile, I looked at it and "*" is not used.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] yanhongwangg commented on pull request #2460: fix show devices
Posted by GitBox <gi...@apache.org>.
yanhongwangg commented on pull request #2460:
URL: https://github.com/apache/iotdb/pull/2460#issuecomment-768871649
> ignore the last `.*`.
Maybe i can fix it in show devices by ignoring the last .*., but there is another question
before ignoring the last .*., when i execute show devices root.a.b.* ,the result is root.a.b.c,
after ignoring the last .*., execute the same sql ,it will match root.a.b and root.a.b.c,
result is inconsistent.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] yanhongwangg commented on pull request #2460: fix show devices
Posted by GitBox <gi...@apache.org>.
yanhongwangg commented on pull request #2460:
URL: https://github.com/apache/iotdb/pull/2460#issuecomment-758349189
> Good job, Yanhong!
> Consider add an UT or IT?
OK, I'll add 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org