data: support duplicate Field names in a Frame #83

kylebrandt · 2020-03-09T18:24:05Z

Add fieldIdx prefix to arrow field Name to keep the names unique.

Front-end will need to do the same removal of the prefix.

ryantxu · 2020-03-09T18:46:51Z

Is the proposal here to always have the prefix number? Maybe we can check and append (if necessary) before making a frame?

kylebrandt · 2020-03-09T18:52:56Z

Idea is always as part of the to/from arrow process.

Has to be always, or can't tell if it is our prefix or name that looks like our prefix.

ryantxu · 2020-03-09T20:01:28Z

Can we add a constraint that the names must be unique? And add a helper to append indicies when necessary? (Then we don't need to modify on the read side)

kylebrandt · 2020-03-09T20:07:46Z

Can we add a constraint that the names must be unique?

Can do it via documentation, and maybe helper functions. However, this awkward is standard wide time series response. You have a metric name that would be repeated, but different Labels on the frame for that repeated name.

And add a helper to append indicies when necessary? (Then we don't need to modify on the read side)

I don't understand what you are saying here.

kylebrandt · 2020-03-09T20:09:48Z

Also this means that Tables that don't have names, and just positions can't be represented. Because Name: "" - empty string, would still be a duplicate.

ryantxu · 2020-03-09T21:59:09Z

If I understand this PR, it will throw an error for any dataframe that does not have a field name that matches "${index}: ${string}" -- that seems pretty rough.

Perhaps the long/wide syncing could assume that fields named:
value, value (1), value (2) are all meant to be called "value"

with changing the name to "${index}: ${string}" it seems we may as well name it a UUID and get the name from elsewhere

kylebrandt · 2020-03-09T22:03:39Z

I like the UUID idea and just using metadata for name in arrow. Can work on that tomorrow.

ryantxu · 2020-03-09T22:30:36Z

i am reluctant to have the names be totally arbitrary -- if we need a mode where they are, that seems fine, but name==UUID seems weird.

This would be the first choice that makes it so an arbitrary arrow file would not "just work"

ryantxu · 2020-03-09T22:42:23Z

If I understand the feature request in this PR is that you want to be able to represent multiple fields with the same name in arrow Table, not just the fields?

kylebrandt · 2020-03-09T22:55:58Z

I don't understand the "not just the fields" part...

But yes, the idea is that a Dataframe supports multiple Fields with the same Name, so the name doesn't have to be unique (or set) within a dataframe.

kylebrandt · 2020-03-10T12:57:40Z

Consider:

MySQL > select 1 AS a, 2 AS a;
+---+---+
| a | a |
+---+---+
| 1 | 2 |
+---+---+

ryantxu

LGTM -- what else needs updated in core before while we bump the version?

ryantxu · 2020-03-10T19:52:18Z

data/arrow.go

+	if len(sp) == 2 {
+		return sp[1]
+	}
+	return name


marefr · 2020-03-10T19:55:38Z

I don't really have any preferences, but if arrow have this restriction I don't see why we shouldn't have this resctriction.
MySQL > select 1 AS a, 2 AS a;
Even id you could do this it doesn't make sense to me. A table in database cannot have 2 columns with the same name so unless you do something crazy suit yourself is my point 😄

kylebrandt · 2020-03-10T20:00:25Z

I don't really have any preferences, but if arrow have this restriction I don't see why we shouldn't have this resctriction.
MySQL > select 1 AS a, 2 AS a;
Even id you could do this it doesn't make sense to me. A table in database cannot have 2 columns with the same name so unless you do something crazy suit yourself is my point

There are less abnormal examples. For example a CSV file with no headers (name is absent), or maybe everything in the frame is the same "name", so the frame gets a name, but you want to leave all the column names default ("").

kylebrandt · 2020-03-10T20:01:33Z

LGTM -- what else needs updated in core before while we bump the version?

I can't think so. Once you have a Frontend PR ready that goes with this. I will merge it, tag it, and then update your PR to use this.

marefr · 2020-03-10T20:12:22Z

I don't really have any preferences, but if arrow have this restriction I don't see why we shouldn't have this resctriction.
MySQL > select 1 AS a, 2 AS a;
Even id you could do this it doesn't make sense to me. A table in database cannot have 2 columns with the same name so unless you do something crazy suit yourself is my point

There are less abnormal examples. For example a CSV file with no headers (name is absent), or maybe everything in the frame is the same "name", so the frame gets a name, but you want to leave all the column names default ("").

Sure. The question is how Grafana should be able to handle it if the idea is that Grafana should remove the identifier. Can Grafana handle two fields named the same?

ryantxu · 2020-03-10T23:03:30Z

Can Grafana handle two fields named the same?

Yes -- there is a warning that if you have two fields with the same name some things -- like using template variables for display (ie: ${frame.name[2]}) won't work

bergquist

👍

❤️ golen test files

marefr · 2020-03-11T09:47:06Z

Can Grafana handle two fields named the same?

Yes -- there is a warning that if you have two fields with the same name some things -- like using template variables for display (ie: ${frame.name[2]}) won't work

Great. Just want to make sure I understand everything. So what would the field type of a "merged" field be in Grafana - none?

kylebrandt · 2020-03-11T13:27:45Z

Front end change exists it seems: grafana/grafana#22705

This reverts commit 90efbdc.

data.Frame's Field's Name prefixing hack is now no longer needed, Arrow now allows for duplicate Field names, at least in our use cases. * Revert "data: support duplicate Field names in a Frame (#83)" This reverts commit 90efbdc * Update to newer version of go Arrow lib that allows duplicate field names and add a test that panicked before the upgrade. Also add duplicate field to golden file. fixes #137

kylebrandt added 2 commits March 9, 2020 13:58

dataframe: arrow - prefix field names to make them unique

be22f88

for #59

fixes

d42aae2

kylebrandt requested a review from ryantxu March 9, 2020 18:24

kylebrandt added arrow https://godoc.org/github.com/apache/arrow/go/arrow dataframe labels Mar 9, 2020

kylebrandt added this to the 6.7 milestone Mar 9, 2020

kylebrandt added 4 commits March 10, 2020 14:39

do not err if no name prefix, add sloth to prefix per ryan

ff2c244

Merge branch 'master' into df_field_name_prefix

6ac23de

code comments

bb7fbd6

proper const

ead9920

ryantxu approved these changes Mar 10, 2020

View reviewed changes

data/arrow.go

if len(sp) == 2 {

return sp[1]

}

return name

Copy link

Member

ryantxu Mar 10, 2020

Choose a reason for hiding this comment

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

👍

bergquist approved these changes Mar 11, 2020

View reviewed changes

kylebrandt changed the title ~~dataframe: field name prefix~~ data: support duplicate field names in arrow by adding name prefix Mar 11, 2020

kylebrandt changed the title ~~data: support duplicate field names in arrow by adding name prefix~~ data: support duplicate field names in arrow Frame encoding by adding name prefix Mar 11, 2020

kylebrandt changed the title ~~data: support duplicate field names in arrow Frame encoding by adding name prefix~~ data: support duplicate Field names in a Frame Mar 11, 2020

kylebrandt merged commit 90efbdc into master Mar 11, 2020

kylebrandt deleted the df_field_name_prefix branch March 11, 2020 13:31

kylebrandt mentioned this pull request Mar 12, 2020

Work out dataframe unique name restriction due to arrow #59

Closed

kylebrandt mentioned this pull request Apr 15, 2020

Consider Removing Sloth Prefix (🦥: idx) #137

Closed

kylebrandt added a commit that referenced this pull request Apr 15, 2020

Revert "data: support duplicate Field names in a Frame (#83)"

4eb88e8

This reverts commit 90efbdc.

kylebrandt mentioned this pull request Apr 15, 2020

data: remove Field prefix during arrow encoding #138

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

data: support duplicate Field names in a Frame #83

data: support duplicate Field names in a Frame #83

kylebrandt commented Mar 9, 2020 •

edited

ryantxu commented Mar 9, 2020

kylebrandt commented Mar 9, 2020

ryantxu commented Mar 9, 2020

kylebrandt commented Mar 9, 2020

kylebrandt commented Mar 9, 2020

ryantxu commented Mar 9, 2020

kylebrandt commented Mar 9, 2020

ryantxu commented Mar 9, 2020

ryantxu commented Mar 9, 2020

kylebrandt commented Mar 9, 2020 •

edited

kylebrandt commented Mar 10, 2020

ryantxu left a comment

ryantxu Mar 10, 2020

marefr commented Mar 10, 2020

kylebrandt commented Mar 10, 2020

kylebrandt commented Mar 10, 2020

marefr commented Mar 10, 2020

ryantxu commented Mar 10, 2020

bergquist left a comment •

edited

marefr commented Mar 11, 2020

kylebrandt commented Mar 11, 2020

data: support duplicate Field names in a Frame #83

data: support duplicate Field names in a Frame #83

Conversation

kylebrandt commented Mar 9, 2020 • edited

ryantxu commented Mar 9, 2020

kylebrandt commented Mar 9, 2020

ryantxu commented Mar 9, 2020

kylebrandt commented Mar 9, 2020

kylebrandt commented Mar 9, 2020

ryantxu commented Mar 9, 2020

kylebrandt commented Mar 9, 2020

ryantxu commented Mar 9, 2020

ryantxu commented Mar 9, 2020

kylebrandt commented Mar 9, 2020 • edited

kylebrandt commented Mar 10, 2020

ryantxu left a comment

Choose a reason for hiding this comment

ryantxu Mar 10, 2020

Choose a reason for hiding this comment

marefr commented Mar 10, 2020

kylebrandt commented Mar 10, 2020

kylebrandt commented Mar 10, 2020

marefr commented Mar 10, 2020

ryantxu commented Mar 10, 2020

bergquist left a comment • edited

Choose a reason for hiding this comment

marefr commented Mar 11, 2020

kylebrandt commented Mar 11, 2020

kylebrandt commented Mar 9, 2020 •

edited

kylebrandt commented Mar 9, 2020 •

edited

bergquist left a comment •

edited