Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use selected time range for metaqueries in DE #12791

Merged
merged 2 commits into from
Mar 20, 2019
Merged

Conversation

chnn
Copy link
Contributor

@chnn chnn commented Mar 20, 2019

Closes #12779
Closes #11535

reloading

Screen Shot 2019-03-20 at 1 38 54 PM

@chnn chnn force-pushed the ui-de-meta-query-time-range branch from c55ea0e to e062185 Compare March 20, 2019 20:46
@chnn
Copy link
Contributor Author

chnn commented Mar 20, 2019

Connect influxdata/flux#1071

Copy link
Contributor

@alexpaxton alexpaxton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good, just some comments about usage of margin variables in css

justify-content: center;
align-items: center;
padding: 12px 12px 36px 12px;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use margin variables from _variables.scss?

Could probably do something like:

padding: $ix-marg-c;
padding-bottom: $ix-marg-d;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep can do

display: block;
font-size: 14px;
text-transform: none;
margin-top: 10px;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment about variables

@chnn chnn force-pushed the ui-de-meta-query-time-range branch from e062185 to d99d9b9 Compare March 20, 2019 21:31
@chnn chnn merged commit 60ae462 into master Mar 20, 2019
@chnn chnn deleted the ui-de-meta-query-time-range branch March 20, 2019 22:46
chnn added a commit that referenced this pull request Apr 26, 2019
Currently, slight changes in the form of a Flux metaquery can have
drastic performance implications. To solve this issue, the Flux language
provides helper metaquery functions such as `v1.tagKeys` and
`v1.tagValues` which are guaranteed to be as fast as possible.

In #12791, we switched away from using the `v1.tagKeys` and
`v1.tagValues` functions in the query builder to their underlying Flux
implementations in order to implement a UI feature. While the new
metaqueries used in the query builder were still optimized at the time,
the Flux language has since changed and this is no longer the case.

In addition, the metaqueries in the UI no longer hit the same code-path
in Flux which has exposed a logic bug in the queries. So when executed,
the metaqueries return the following error:

    schema collision detected: column ""_value"" is both of type float and int

This PR updates the metaqueries used in the query builder to their
[currently optimized form][0]. The long term solution is to address
[this][1] issue and then switch back to using the safer `v1.tagKeys` and
`v1.tagValues` functions directly.

[0]: https://github.com/influxdata/flux/blob/master/stdlib/influxdata/influxdb/v1/v1.flux
[1]: influxdata/flux#1071

Closes #13660
chnn added a commit that referenced this pull request Apr 26, 2019
Currently, slight changes in the form of a Flux metaquery can have
drastic performance implications. To solve this issue, the Flux language
provides helper metaquery functions such as `v1.tagKeys` and
`v1.tagValues` which are guaranteed to be as fast as possible.

In #12791, we switched away from using the `v1.tagKeys` and
`v1.tagValues` functions in the query builder to their underlying Flux
implementations in order to implement a UI feature. While the new
metaqueries used in the query builder were still optimized at the time,
the Flux language has since changed and this is no longer the case.

In addition, the metaqueries in the UI no longer hit the same code-path
in Flux which has exposed a logic bug in the queries. So when executed,
the metaqueries return the following error:

    schema collision detected: column ""_value"" is both of type float and int

This PR updates the metaqueries used in the query builder to their
[currently optimized form][0]. The long term solution is to address
[this][1] issue and then switch back to using the safer `v1.tagKeys` and
`v1.tagValues` functions directly.

[0]: https://github.com/influxdata/flux/blob/master/stdlib/influxdata/influxdb/v1/v1.flux
[1]: influxdata/flux#1071

Closes #13660
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants