Skip to content

fix show devices#2460

Closed
yanhongwangg wants to merge 1 commit intoapache:masterfrom
yanhongwangg:fix_show_devices
Closed

fix show devices#2460
yanhongwangg wants to merge 1 commit intoapache:masterfrom
yanhongwangg:fix_show_devices

Conversation

@yanhongwangg
Copy link
Contributor

before,when i create a timeseries with three nodes an then execute show devices
image2021-1-9 17_24_25

after,same operation
image

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jan 9, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@HTHou
Copy link
Contributor

HTHou commented Jan 11, 2021

Good job, Yanhong!
Consider add an UT or IT?

@yanhongwangg
Copy link
Contributor Author

Good job, Yanhong!
Consider add an UT or IT?

OK, I'll add it

@jixuan1989
Copy link
Member

ping

@neuyilan neuyilan requested a review from jt2594838 January 22, 2021 01:12
@neuyilan neuyilan added the bug Something isn't working label Jan 22, 2021
@jt2594838
Copy link
Contributor

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.

@yanhongwangg
Copy link
Contributor Author

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.

@jt2594838
Copy link
Contributor

jt2594838 commented Jan 27, 2021

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 .*.

@jt2594838
Copy link
Contributor

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.

@jixuan1989 jixuan1989 added the Module - Cluster PRs for the cluster module label Jan 27, 2021
@yanhongwangg
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working Module - Cluster PRs for the cluster module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants