Cartodb: Map instantiation fails when map is created from shared dataset

[nobuti]

Working on #9516 I faced a trouble related to the query and the map instantiation. Follow theses steps to reproduce it:

• Share a dataset with write permission. Don't qualified the sql query (aka, include the username as part of the table name in the query).
• Using the user that it's shared with, create a map from this dataset.
• 💥 Dashboard has some errors: Error: Map instantiation failed.

This is happening because the queries present in the vizJSON are not qualified with the owner's username.

This is hard to fix in the frontend (in the right way) because the map is initially instantiated in the deep-insight (and cartodb.js) side, from the data coming from the vizJSON. I tried to patch the vizJSON to fix the queries, and it solves the issue, but the solution smells. I feel the queries should come qualified in the vizJSON, in the same manner as table_name properties.

[nobuti]

Of course, qualifying the query manually in the sql editor fixes the issue.

[nobuti]

I forgot to ping @javitonino and @gfiorav, I think we will need your point of view with this.

[javitonino]

Yes, I think this is a better fix in backend. We are doing a lot of query rewriting already anyway. In fact, I think this should be already be handled in some way, but let me check.

[javitonino]

After looking at this, we are not doing query rewriting on map creation at all.

[javitonino]

There are two behaviours right now:

• When adding a new layer, the query is not copied
• When creating a new map from a dataset, the query is copied (which could be wrong)

In fact, in the second case, everything is copied, queries, styles, infowindows, basemaps, etc. Basically, the canonical visualization is duplicated. This made a lot of sense in the editor, where canonical visualizations where editable, but makes less sense in the new builder.

I think we should try to make the two cases above the same, so adding a layer behaves the same way as creating a map from a dataset. So either we copy (and rewrite) the query in both cases or we don't do it at all.

Right now, I'm inclined to think that we should never copy anything from the datasets when creating a map (for the builder, keeping current behaviour for editor). This way, it always behaves as adding a layer and gets the default query. This is also consistent with adding tables in analysis nodes.

The other option would be to copy and rewrite the query always, but it has some complications with current implementation, most notable that the layer creation is done mostly by frontend (backend is just acting as a persistence layer here, and has no say on the process).

[juanignaciosl]

+1 to merge both cases into "add layer" one.

[michellechandra]

S/B: 9695778

[gfiorav]

Did we decide what to do with this one after all? @javitonino @juanignaciosl

[juanignaciosl]

First of all, in order to keep issues linked, this is somehow related to #9478. Not the problem but the implementation.

Answering your question, I think that there hasn't been an explicit agreement. Nevertheless, comments at both issues are aligned: let's do this (1) in backend, (2) during map creation (3) with the "add layer" logic.

_Disclaimer: as you know I still don't love (1), but there's no clear winner and @xavijam and @javisantana have a broader vision on this_

@javitonino , @gfiorav , agree?

[javitonino]

Agreed. I dislike having add layer creating the layer options in frontend and map creation doing it in backend, but I think the best solution for doing thing at creation time is in the backend.

Note that for this issue we will have to check if the builder is enabled or not to toggle the behaviour, as we want to keep the old behaviour (copy map with TableBlender) for the editor.

[javitonino]

Final decision after some offline discussion. On map creation we will reset the styles from the dataset, but keep the query that's been applied (in order to avoid surprises when going from the dataset preview to create a new map). So basically, we are just keeping the query, rewritten. Of course, this is toggled by builder activation.

This is being done in #9749 (queries) and #9726 (styles).

[xavijam]

:+1: