Data Explorer `int_list` replaces 1st element with 0

Not sure if this is a bug or whether I’m doing something wrong, but it seems the first element in an int_list array is always replaced with 0 in a data explorer query.

Eg:

-- int_list :categories = "3, 5, 6"

gets changed to [0, 5, 6]

looking at:

https://github.com/discourse/discourse-data-explorer/blob/master/plugin.rb#L942

If I do that in console I get the expected output:

string = "3,5,6"
value = string.split(',').map { |s| s.downcase == '#null' ? nil : s.to_i }
=> [3, 5, 6]

You can reproduce with this query:

-- [params]
-- int_list :categories = "3, 5, 6"

SELECT *
FROM topics 
WHERE topics.category_id = ANY (ARRAY [ :categories ]  )

To break it (so you can view the error) simply remove the closing bracket, eg:

-- [params]
-- int_list :categories = "3, 5, 6"

SELECT *
FROM topics 
WHERE topics.category_id = ANY (ARRAY [ :categories   )
PG::SyntaxError: ERROR:  syntax error at or near ")"
LINE 12: WHERE topics.category_id = ANY (ARRAY [ 0,5,6   )
                                                         ^

(Look at the array)

1 Like

It looks like what is happening when you set a default value for an int_list parameter in this way:

-- int_list :categories = "3, 5, 6"

is that the following gets run:

'"1'.downcase.to_i

That will return 0.

You can get around the issue by leaving the quotes off the default parameter:

-- int_list :categories = 3, 5, 6

That value still gets interpreted as a string before being split into an array. Maybe the plugin should strip outer quotation marks if they get added to a string input.

6 Likes

Thanks Simon! :smiley:

I was sure I had tried that… and looking again now I see that changing default parameters in the body and clicking on save and run has no effect - you either have to refresh the page after saving or change the values in the box for the field as well (which I totally forgot about).

2 Likes

FWIW I find params really fiddly to reason about, I wonder if we should shift this to full UX vs magic comments @riking

5 Likes

Yup, totally makes sense to do that, the magic comments were optimizing for minimum # of plugin-sourced DB migrations, which are much less of a problem these days.

5 Likes