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

dedupe x-axis values more carefully #4677

Merged
merged 3 commits into from
Aug 18, 2015
Merged

Conversation

spalger
Copy link
Contributor

@spalger spalger commented Aug 15, 2015

Fixes #4670

In #4613 the range buckets are no longer represented by a string, but by an object. This enhances support for such keys by:

  • Always returning the same/=== key for buckets which are a part of the same range
  • Supporting string serialization (for d3 domain support)
  • Using a map, rather than an object, for deduping values in vislib/components/zero_injection/uniq_key.js

@spalger spalger changed the title [vislib/xAxis] dedupe xValues more carefully, allowing object keys dedupe x-axis values more carefully Aug 15, 2015
@w33ble
Copy link
Contributor

w33ble commented Aug 17, 2015

Fix works, LGTM.

@lukasolson
Copy link
Member

  1. Create a bar chart
  2. Create a range x-axis (or split bars) aggregation on a scripted field
  3. Click on one of the bars to filter

I get the following error:

Error: Request to Elasticsearch failed: {"error":{"root_cause":[{"type":"expression_script_compilation_exception","reason":"Failed to parse expression: (abs(doc['amount'].value))>=gte && (abs(doc['amount'].value))<lt && (abs(doc['amount'].value))function toString() { [native code] }toString && (abs(doc['amount'].value))undefinedvalue","stack_trace":"ExpressionScriptCompilationException[Failed to parse expression: (abs(doc['amount'].value))>=gte && (abs(doc['amount'].value))<lt && (abs(doc['amount'].value))function toString() { [native code] }toString && (abs(doc['amount'].value))undefinedvalue]; nested: ParseException[ missing 'function' at position (94).]; nested: MissingTokenException;\n\tat org.elasticsearch.script.expression.ExpressionScriptEngineService.compile(ExpressionScriptEngineService.java:98)\n\tat

I tried this on master (with split bars, cuz obviously x-axis isn't working with range) and didn't get the same error.

@lukasolson lukasolson assigned spalger and unassigned lukasolson Aug 17, 2015
@spalger spalger force-pushed the fix/4670 branch 2 times, most recently from 5eb8070 to 8a392f9 Compare August 18, 2015 04:06
@spalger
Copy link
Contributor Author

spalger commented Aug 18, 2015

@lukasolson at some point the bucket key is passed as params and itterated as though every key is a parameter name. This seems fair to me, so I made #toString() non-enumerable by creating a little RangeKey class. This also presented an issue with the way that scripted fields are formatted in the filter bar, which I took care of really quick.

@spalger spalger assigned lukasolson and unassigned spalger Aug 18, 2015
@lukasolson
Copy link
Member

LGTM.

lukasolson added a commit that referenced this pull request Aug 18, 2015
dedupe x-axis values more carefully
@lukasolson lukasolson merged commit c9e8895 into elastic:master Aug 18, 2015
@w33ble w33ble mentioned this pull request Sep 4, 2015
@spalger spalger deleted the fix/4670 branch September 4, 2015 21:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants