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

CSV node msg.columns bug #2853

Closed
5 tasks
fritzvdw opened this issue Feb 1, 2021 · 14 comments · Fixed by #2854
Closed
5 tasks

CSV node msg.columns bug #2853

fritzvdw opened this issue Feb 1, 2021 · 14 comments · Fixed by #2854

Comments

@fritzvdw
Copy link

fritzvdw commented Feb 1, 2021

What are the steps to reproduce?

node.template is not getting cleaned if you pass in msg.columns with different values

I am reading files with json object that I want to use CSV node to convert to string - this file can be changed to have different columns values and is then parsed by same CSV node
I have a function node before the csv node that assigns msg.columns from the payload dynamically

pass in say 8 columns for example msg.columns = "col1,col2,col3,col4,col5,col6,col7,col8"
and then pass in another file with 10 columns msg.columns = "col1,col2,col3,col4,col5,col6,col7,col8,col9,col10"

when the node is run the second time (without re-deploying - note deploying resets this) - then it keeps the first node.template value and keeps overriding msg.columns and parses the payload incorrectly.

This is the line that I believe has the bug:
if ((node.template.length === 1) && (node.template[0] === '') && (msg.hasOwnProperty("columns"))) {

This is what I think it should be - to allow that the node.tempalte value gets reset if there is a msg.columns
if ((node.template.length === 1) && (node.template[0] === '') || (msg.hasOwnProperty("columns"))) {

What happens?

when the node is run the second time (without re-deploying - note deploying resets this) - then it keeps the first node.template value and keeps overriding msg.columns and parses the payload incorrectly.

What do you expect to happen?

It should not override the msg.columns and get the payload converted incorrectly

Please tell us about your environment:

  • Node-RED version: 1.2.7
  • Node.js version: v13.9.0
  • npm version: 6.13.7
  • Platform/OS: Windows
  • Browser: Chrome
@fritzvdw
Copy link
Author

fritzvdw commented Feb 1, 2021

or perhaps this is needed
ou = ou.slice(0, -1) + node.ret;
}
else {
if (msg.hasOwnProperty("columns")) {
node.template = clean((msg.columns || "").split(","));
}

@dceejay
Copy link
Member

dceejay commented Feb 1, 2021

If you were writing this to a file and suddenly changed columns half way through this would be bad....

@fritzvdw
Copy link
Author

fritzvdw commented Feb 1, 2021

Perhaps my description is not clear - but I am not writing it to a file, easiest or quickest way to replicate it is to read a file, because I can change the file to have different columns which is when this issue happens (without changing inside node-red and deploying again). But the reading or writing files has nothing to do with the issue really - so not sure why you are commenting on that

@dceejay
Copy link
Member

dceejay commented Feb 1, 2021

because that was the use case envisioned when it was created. If you are creating a CSV you would expect the columns to remain constant. I can see your use case is different, so this is a new feature rather than an issue per se.

@fritzvdw
Copy link
Author

fritzvdw commented Feb 1, 2021

Okay, I hear you on writing to file and how you planned it initially, but CSV is a parser node that parses object data to string, for which I think you should be able to pass it msg.columns the way you would want. Thanks

@dceejay
Copy link
Member

dceejay commented Feb 1, 2021

I hear you.

@dceejay
Copy link
Member

dceejay commented Feb 1, 2021

OK - need some more clarity... - You are reading a file... within that file surely the fields that you need to map to columns remain the same ? so once defined they are constant for that file ? If so then the issue is that the template isn't being reset at the end of that file ? If so how are you reading in the file ? Is it using a file in node set to read one line at a time ? (so that you can then parse the json etc..)

Or are you actually wanting to change the columns on a per row basis ?

@fritzvdw
Copy link
Author

fritzvdw commented Feb 2, 2021

The input is an array of objects that I need to be parsed as a multi line CSV string. This input can have different object properties and will always have 2 or more objects in the array, and the properties for each (columns for csv string) can or will differ.

Example:

  1. [{"col1":"header1","col2":"header2"},{"col1":"line1"}]
    desired output:
    header1,header2
    line1
    (line1, is also fine)
  2. [{"col1":"header1"},{"col1":"line1","col2":"line2"}]
    header1
    line1,line2
    (header1, is also fine)
    when example 2 runs without msg.columns it cuts of line2 and gives
    header1
    line1
    To overcome this I pass msg.columns="col1,col2" then example 1 and 2 will work fine

However when after this there is another value passed in as
3)[{"col1":"header1","col2":"header2","col3":"header3"},{"col1":"line1","col2":"line2"}]
it outputs as
header1,header2
line1,line2
even if I pass msg.columns as "col1,col2,col3" - because within the node it changes msg.columns back to "col1,col2"
(note - this 3 examples happen without redeploy of node-red as re-deploying changes this behavior - and resets the msg.columns value that is being changed back)

@dceejay
Copy link
Member

dceejay commented Feb 2, 2021

OK - so I'm working on a fix... can you confirm the expected outputs for your examples ? This is without setting any msg.columns of course... (and sending column headers (which can be turned off)

1

col1,col2
header1,header2
line1,

2

col1
header1
line1

3

col1,col2,col3
header1,header2,header3
line1,line2,

@fritzvdw
Copy link
Author

fritzvdw commented Feb 2, 2021

example 2 should be
col1,col2
header1,
line1,line2

need it to not cut off any data
the other examples would work fine

@dceejay
Copy link
Member

dceejay commented Feb 2, 2021

I think I'm going to push back on that... again if you don't set the headers up front - how does it know where to add an extra column - if say you had example like

[
    {
        "col1": "header1",
        "col2": "header2",
        "col3": "header3",
        "col4": "header4"
    },
    {
        "col1": "itemA",
        "col2": "itemB"
    },
    {
        "col1": "itemA",
        "col3": "itemC"
    },
    {
        "col1": "itemA",
        "col4": "itemD"
    }
]

you would get correct csv like

col1,col2,col3,col4
header1,header2,header3,header4
itemA,itemB,,
itemA,,itemC,
itemA,,,itemD

If you reset it on every line you would get

col1,col2,col3,col4
header1,header2,header3,header4
itemA,itemB
itemA,itemC
itemA,itemD

which would be plain wrong

This is in the context of sending in an array - of course if sent as single lines then there is no context of what came before so it would break as you want :-)

@fritzvdw
Copy link
Author

fritzvdw commented Feb 2, 2021

I think I'm going to push back on that... again if you don't set the headers up front - how does it know where to add an extra column

I do want to set the headers in msg.column - but this cannot then be changed when node is called again? it keeps this, even if you pass something new it does not change

[
    {
        "col1": "header1",
        "col2": "header2",
        "col3": "header3",
        "col4": "header4"
    },
    {
        "col1": "itemA",
        "col2": "itemB"
    },
    {
        "col1": "itemA",
        "col3": "itemC"
    },
    {
        "col1": "itemA",
        "col4": "itemD"
    }
]

I would not have an example like this really if there is only 2 items it would be col1 and col2 and not for example col4 - then there would need to be 4 properties

This is in the context of sending in an array - of course if sent as single lines then there is no context of what came before so it would break as you want :-)

But this sounds like then I might as well parse everything in a function node

@dceejay
Copy link
Member

dceejay commented Feb 2, 2021

yes the fix resets for every new array or file. and honours msg.columns, so you can now call multiple times in any order.

@fritzvdw
Copy link
Author

fritzvdw commented Feb 2, 2021

Great, thanks!

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 a pull request may close this issue.

2 participants