?_nocol= does not interact well with default facets

[simonw]

Clicking "Hide this column" on fips on https://covid-19.datasettes.com/covid/ny_times_us_counties shows this error:

https://covid-19.datasettes.com/covid/ny_times_us_counties?_nocol=fips

Invalid SQL

no such column: fips

The reason is that https://covid-19.datasettes.com/-/metadata sets up the following:

  "ny_times_us_counties": {
      "sort_desc": "date",
      "facets": [
          "state",
          "county",
          "fips"
      ],

It's setting fips as a default facet, which breaks if you attempt to remove the column using ?_nocol.

[simonw]

Related issue: visit https://latest.datasette.io/fixtures/facetable?_facet=state and click "Hide this column" on the "state" cog menu and you get https://latest.datasette.io/fixtures/facetable?_facet=state&_nocol=state which shows an error:

Invalid SQL

no such column: state

[simonw]

Two options here:

• Don't provide users with options that will lead to this situation - so no "Hide this column" option on pages that are already faceted by that column
• Ignore facet selections for columns which are no longer visible

I think I like the first option more.

I could partially implement that in the table.js JavaScript by looking at the ?_facet= parameters... but that won't cover the case where the facet is happening because of default facets configured in metadata.yml.

Instead the JavaScript should look for evidence in the DOM that specific facets are enabled. This could also help me cover other types of faceting, such as ?_facet_array= or even custom facets provided by plugins.

[simonw]

Maybe there's a concept here of the columns that are required by a selected facet? Those can then be included as data- attributes on the page, which will then impact which "Hide this column" options are available.

I can also use them to provide a better error message than "no such column: state" - I can verify that ?_nocol and ?_col have not been used to disable the required columns.

There is one other option here: I could still include the columns that are known to be needed for faceting in the faceting SQL queries, but leave them out of the query that is used to return the results! That's actually a pretty tempting (albeit more complex) option.

[simonw]

Maybe there's a short-term and longer-term solution for this - where the long-term solution is to use different columns in the faceting selects, while the short-term solution is to disable "Hide this column" for certain things.

[simonw]

That long-term solution may not be too difficult. The facets are calculated against sql_no_limit which is constructed here:

datasette/datasette/views/table.py

Lines 659 to 665 in 7b106e1

	sql_no_limit = "select {select} from {table_name} {where}{order_by}".format(
	select=select,
	table_name=escape_sqlite(table),
	where=where_clause,
	order_by=order_by,
	)
	sql = f"{sql_no_limit.rstrip()} limit {page_size + 1}{offset}"

And used here:

datasette/datasette/views/table.py

Lines 706 to 718 in 7b106e1

	for klass in facet_classes:
	facet_instances.append(
	klass(
	self.ds,
	request,
	database,
	sql=sql_no_limit,
	params=params,
	table=table,
	metadata=table_metadata,
	row_count=filtered_table_rows_count,
	)
	)

Crucially, sql_no_limit is ONLY used for faceting - nothing else uses it anywhere. So constructing it before constructing sql and taking ?_col= and ?_nocol= into account may not be a complex change.

[simonw]

Yes! This was easier than I thought. I'm going with that solution - where facets are calculated against all columns, ignoring ?_col= and ?_nocol= entirely.

[simonw]

Demo: https://latest.datasette.io/fixtures/facetable?_facet=state&_nocol=state - the state column is not selected but facet by state still works: