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

1.4 doesn't allow binary operations on literals #9290

Closed
PavelSarek opened this issue Jan 8, 2018 · 17 comments
Closed

1.4 doesn't allow binary operations on literals #9290

PavelSarek opened this issue Jan 8, 2018 · 17 comments
Assignees
Labels
Milestone

Comments

@PavelSarek
Copy link

Bug report

System info: Any 1.4.x or the last 1.5 nightly build

Steps to reproduce:

Execute query:
select count * (1 / 1) from measurement
or
select * from measurement where 'a'='a'

Expected behavior: The same as in the previous versions, i.e. 1.3.x and older. The expression which can be reduced to a constant during AST processing is accepted and correctly evaluated. 'a'='a' would be equivalent to true, count * (1 / 1) would be the same as count * 1

Actual behavior: The expression is refused by the parser. With ERR: cannot perform a binary expression on two literals, or respectively ERR: invalid expression: 'a' = 'a'

Additional info:
Might be related to #9226

Rule binary_op in InfluxQL reference .

Use case: Some use-cases in Grafana using templating variables.

The first query is derived from autoscaling implementation:

SELECT sum("count")/($__interval_ms/3600000) GROUP BY time($__interval)

the second is used to add some modes to Grafana dashboard by switching some queries on/off based on templating variables.

@prokod
Copy link

prokod commented Jan 9, 2018

@rbetts @jsternberg Could you please have a look on that ? Thx

@jsternberg
Copy link
Contributor

@rbetts this seems like a candidate for backporting.

@rbetts
Copy link
Contributor

rbetts commented Jan 12, 2018

@jsternberg approved.

jsternberg added a commit to influxdata/influxql that referenced this issue Jan 12, 2018
This allows literals to be evaluated within the condition expression.

Fixes influxdata/influxdb#9290.
@ghost ghost removed the proposed label Jan 18, 2018
@jsternberg jsternberg reopened this Jan 18, 2018
@ghost ghost assigned jsternberg Jan 18, 2018
@ghost ghost added the review label Jan 18, 2018
jsternberg added a commit that referenced this issue Jan 18, 2018
jsternberg added a commit that referenced this issue Jan 18, 2018
@ghost ghost removed the review label Jan 18, 2018
@jasonbogdanski
Copy link

@PavelSarek Did you happen to find a workaround for auto scaling in Grafana? This is still an issue in Influxdb 1.4.3.

@PavelSarek
Copy link
Author

@jasonbogdanski I'm afraid I have no workaround. We reverted to InfluxDB 1.3.7 for now.

@jasonbogdanski
Copy link

jasonbogdanski commented Feb 12, 2018

Thanks @PavelSarek, we're going to have to do the same thing.

@jsternberg jsternberg reopened this Feb 12, 2018
@jsternberg
Copy link
Contributor

This should be fixed in 1.4.3. @PavelSarek can you give me a reproducer?

I also just tested the example in the first post and it appears like this query returned zero results in 1.3.7. But, I'm not getting a parsing failure.

@jasonbogdanski
Copy link

@jsternberg I'm running a docker image of influxdb v1.4.3 from https://hub.docker.com/_/influxdb/. I can reproduce the issues using that version with select count / (1 / 1) from measurement. This issue was also posted again on #9273, which was tagged for a 1.6.0 release. However, I also seen it tagged on the v1.5.0 release.

@benjarrell
Copy link

benjarrell commented Feb 12, 2018

@jsternberg I tried with 1.4.3 and a nightly (1.6.0~n201802060800) RPMs and still gives the same error using SELECT non_negative_derivative(mean("bytes_sent"), 10s) * 8 / (10000 / 1000) FROM "measurement"

@andyxning
Copy link

This is quit a non-backward compatible change. And it should be fixed ASAP.

@rbetts
Copy link
Contributor

rbetts commented Feb 15, 2018

@andyxning we agree. This will be fixed and backported.

@jasonbogdanski
Copy link

jasonbogdanski commented Feb 19, 2018

@rbetts When do you expect this will be fixed and backported?

@andyxning
Copy link

andyxning commented Feb 23, 2018

@rbetts Seems this is a regression issue. Any ETA about this and which release will have this fixed? Without this fixed, we can not upgrade inflxudb to a newer 1.4.x version. Or has this been fixed on the master branch?

@jsternberg
Copy link
Contributor

To clarify, there are two behaviors here that are both bugs. This first one:

select count * (1 / 1) from measurement

This was fixed and backported to 1.4.3.

This one:

select * from measurement where 'a'='a'

This did not produce an error in 1.3, but it also didn't work correctly to begin with. So the regression, which was it causing an error, was fixed and backported to 1.4.3. I've put up a few pull requests to fix the actual behavior so a true literal returns everything. This should be present in 1.5.0 and we will backport it to the 1.4.x line (so likely 1.4.4).

@andyxning
Copy link

andyxning commented Feb 27, 2018

Thanks for you explanation @jsternberg . Any ETA about the release of 1.4.4 with this regression fixed. I am willing to give it a try. :)

@rbetts
Copy link
Contributor

rbetts commented Feb 27, 2018

@andyxning we are wrapping up a 1.5.0 (we built 1.5.0rc4 have been testing internally). This fix will make the 1.5.0 final which should be available on OSS this week if all goes well.

@andyxning
Copy link

@rbetts Good news. 👍

@ghost ghost removed the review label Feb 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants