Aggressive linting of queries - bug fix or feature improvement

,

Over zealous linting metric for queries

The new linting rules can be helpful (open your Debug window and click on the linter tab). But there is one rule that I would like to see tweaked to reduce the number of false positives.

The example:

Background
The linter suggests that the number of columns is potentially a performance issue, but there are several of issues with this metric:

  • Postgres will easily handle large numbers of columns
  • Most applications will have large numbers of columns
  • The Retool way of building a table and form requires you have all the columns in the table or the form won't be fully editable.
  • The metric isn't accounting for the field types and sizes
  • The metric doesn't account for how many rows are pulled or if caching is on.

I see that Retool is trying to be helpful with their linter optimisations. But they have based this one on an overly simplistic metric: measuring column count.

IMHO
I'd like to propose that the linting metric should be based on the sum (in bytes) of all the columns (and yes, you could average the content in string fields).

For example, my nine column query consists of INT, INT, INT, INT, DATE, DATE, DATE, DATE, CHAR(50) which is a tiny 114 bytes per row. Hardly something that needs optimisation.

I'd love for the dev team to consider this so I can keep reducing my linter messages to zero.

2 Likes

Thanks @stewart.anstey :wave:

I just added your feedback to our log of improvement for linting. Appreciate the feedback as always!

Can I piggy back on this and make a note of all these lint errors that I cannot clear and are inherently not errors since Retool includes these.

All the code works, and the lint error is only in the debug window, not in the code itself.

Wow, that's is a lot of linting errors! Thank you for surfacing this. I'll share it with our eng team and push for some movement here.

1 Like

Hi @khill-fbmc,

Sorry to hear about your linting woes!

It's odd to be getting errors for built in modules/functions like moment, formatDataAsArray and formatDataAsObject.

If you have any examples you can share it would be interesting to see the supposedly offending code.

1 Like

Of course:

PieChart_Attendance_Week_Data is almost identical to that (I know! not DRY)

Hi @khill-fbmc ,

I imagine you have fixed this by now! I can replicate it at this end.

chatGPT thinks that the transformer editor probably does not recognise lodash.

The solution is to explicitly define _ at the start of the file.

const _ = window._

This seems to work, but there is probably a better way.

I'm not following, what does ChatGPT have to do with the linter?

Edit: and, no, I do not have it fixed. I made a bookmarklet to clear them from the linter when I need to open it.

javascript: (() => { document.querySelectorAll("div[data-testid^='Problems']").forEach(node => {   const text = node.innerText;   const lodashProblems = /'_' is not defined/.test(text);   const momentProblems = /'moment' is not defined/.test(text);   const retoolProblems = /'formatDataAs(Array|Object)' is not defined/.test(text);   if (lodashProblems || momentProblems || retoolProblems) {     node.remove();   } }) })();

It's still in our queue to be fixed :disappointed: It does seem to be scoped to transformers (vs js queries). In addition to solving the bug, we also have a request to add filters to the linting tab.

For the table columns, I think the idea was to nudge folks that are passively selecting all columns (select * ) to still be mindful of performance if they don't actually need all of the columns, but I agree with your feedback that it could be quite a bit more nuanced! We'll update this thread if our team is able to prioritize a better solution

Suggesting the hint at * makes sense, because I do that often. I am lazy efficient and don't usually want to type out the columns I want. :laughing:

1 Like

:grin: same! very efficient

@stewart.anstey great feedback.

Do you have any more asks for performance lints (the ones with a stopwatch icon like "table has too many cols")? Or the performance tab in debug tools?

We're planning to do a pass on these in Q4. Will definitely fix this lint.

1 Like

I'm betting it might be tricky to implement, but I would love love love if the debug window could be un-docked from the editor window it is in.

I find myself moving it around and resizing it to see what I am doing under it, but if I could move it to my second monitor... :exploding_head:

1 Like