influxdata / flux

Flux is a lightweight scripting language for querying databases (like InfluxDB) and working with data. It's part of InfluxDB 1.7 and 2.0, but can be run independently of those.
https://influxdata.com
MIT License
767 stars 154 forks source link

Using Array Style Access to Object with Variable Fails #2510

Closed rawkode closed 5 months ago

rawkode commented 4 years ago

Apologies for the terrible title, I'm not sure how to describe this except by pasting the code:

Accessing an object through square parens works fine with strings, such as r["value"].

Example:

from(bucket: "metrics")
  |> range(start: v.timeRangeStart, stop: v.timeRangeStop)
  |> filter(fn: (r) => r._measurement == "docker_hub")
  |> filter(fn: (r) => r._field == "pull_count")
  |> map(fn: (r) => ({ r with new_value: r["_value"] }))

However, if we pass in a variable for dynamic access - this fails with type error 7:44-7:57: string != int

key_name = "_value"

from(bucket: "metrics")
  |> range(start: v.timeRangeStart, stop: v.timeRangeStop)
  |> filter(fn: (r) => r._measurement == "docker_hub")
  |> filter(fn: (r) => r._field == "pull_count")
  |> map(fn: (r) => ({ r with new_value: r["${key_name}"] }))
jpacik commented 4 years ago

@rawkode records are not dynamic. Although you can access a record's properties using square parens, r["value"] is translated as r.value. Using anything other than a string literal inside square parens when accessing a record will result in a type error.

rickspencer3 commented 4 years ago
  1. Is this by design?
  2. The error message seems to be totally unrelated to the problem, at least from a user's point of view.
nat45928 commented 4 years ago

I hit this today as well. This would be a very welcome improvement.

jpacik commented 4 years ago

@rickspencer3 to answer your questions:

  1. No this is not by design. Square brackets should be used for array access only, not for objects. It was an early addition to the language that is no longer useful/relevant. It should be removed.
  2. In the second example where a dynamic value is used to access the object, the type checker thinks that r must be an array, and so it determines that the type within the square brackets should be an integer not a string. The error message can be improved by providing this additional context to the user.
sanderson commented 4 years ago

@jlapacik I wouldn't say that it's no longer useful or relevant. I agree that it shouldn't be the primary method for reading object properties, but just recently, we've seen a handful of community use cases where object property keys (in these specific cases, tag keys) contain whitespace and/or special characters. These don't work with dot notation (currently), so bracket notation is the only way to reference those object properties.

r = {tag1: "value1", tag2: "value2", "tag with spaces": "value3", "tag.with.period": "value4"}

// won't work
r.tag with spaces

// won't work
r."tag with spaces"

// Works
r.["tag with spaces"]
// Returns value3
blueforesticarus commented 2 years ago

I ran into this today. The error messages were entirely cryptic and it wasn't until digging through basically all the flux documentation that I got to the one line mentioning that access has to be static. The docs for defining functions don't mention it at all despite tons of standard functions like drop, difference, etc. taking columns as an arg.

How are you supposed to abstract a function without dynamic access to columns?

ex) If I want to make a "subtract function" to subtract two columns and make a third

subtract = (tables=<-, c1, c2) => tables 
  |> pivot(rowKey:["_time"], columnKey: ["_field"], valueColumn: "_value")
  |> map(fn: (r) => ({r with _value: r[c1] - r[c2] })) *NOT POSSIBLE*
  |> drop(columns:[c1,c2])

This seems like a pretty basic usecase for defining functions?

Marwes commented 2 years ago

@blueForestIcarus The experimental record.get function can be used right now. Supporting this in a more stable way or with syntax would require more work as the type system isn't quite ready to handle indexing to work with both arrays + integers and records + strings (labels) https://github.com/influxdata/flux/pull/4716

blueforesticarus commented 2 years ago

Are all the functions which take column names as args not written in flux itself? And is there a performance penalty to using record.get over the static access?

Also, this is probably something that should be addressed in the function definition documentation.

Marwes commented 2 years ago

Are all the functions which take column names as args not written in flux itself?

They are written in go as the type system does not (yet) support them (or they are just more performant when written in go).

And is there a performance penalty to using record.get over the static access?

Likely, yes. Though in this case I guess there isn't a way to use static access(?)

blueforesticarus commented 2 years ago

Likely, yes. Though in this case I guess there isn't a way to use static access(?)

Perhaps it would be better to rename the fields to static names before the pivot.

github-actions[bot] commented 6 months ago

This issue has had no recent activity and will be closed soon.