If you apply ?_facet_array=tags then &_facet=tags does nothing

[simonw]

Start here: https://v0-30-2.datasette.io/fixtures/facetable?_facet_array=tags

Note that tags is offered as a suggested facet. But if you click that you get this:

https://v0-30-2.datasette.io/fixtures/facetable?_facet_array=tags&_facet=tags

The _facet=tags is added to the URL and it's removed from the list of suggested tags... but the facet itself is not displayed:

The _facet=tags facet should look like this:

[simonw]

Implementing #1552 has made a fix for this bug even more important.

[simonw]

I think the problem here may be in the design of the JSON returned by facets. It looks like this:

  "facet_results": {
    "tags": {
      "name": "tags",
      "type": "array",
      "results": [...],
      "hideable": false,
      "toggle_url": "/fixtures/facetable.json?_facet=tags&_trace=1&_nosuggest=1",
      "truncated": false
    },
    "created": {
      "name": "created",
      "type": "date",
      "results": [...]

The problem then is that the tags key is over-ridden by the second facet with a different type against the same column name!

https://latest-with-plugins.datasette.io/fixtures/facetable?_trace=1&_facet=created&_facet_date=created&_facet_array=tags&_facet=tags confirms that the SQL queries for those facets are being executed - but the final JSON doesn't show them on https://latest-with-plugins.datasette.io/fixtures/facetable.json?_trace=1&_facet=created&_facet_date=created&_facet_array=tags&_facet=tags

They're not available in the template context either: https://latest-with-plugins.datasette.io/fixtures/facetable?_facet=created&_facet_date=created&_facet_array=tags&_facet=tags&_context=1

[simonw]

The big question here is do I break any existing clients of the "facet_results" JSON API?

It's still pre-1.0 so I could break them, but I've also built my own code against this in the past so it's likely other people have too.

If I don't break them, I will instead need to come up with a naming convention for those keys - something like "tags__array" for example. As well as a way to ensure that a column called tags__array doesn't end up conflicting with the tags__array key!

[simonw]

Datasette's own HTML rendering code doesn't actually use the keys in facet_results - it instead loops through sorted_facet_results which is defined like this:

datasette/datasette/views/table.py

Lines 937 to 941 in 992496f

	"sorted_facet_results": sorted(
	facet_results.values(),
	key=lambda f: (len(f["results"]), f["name"]),
	reverse=True,
	),

And used like this:

datasette/datasette/templates/table.html

Lines 154 to 156 in 992496f

	{% if facet_results %}
	<div class="facet-results">
	{% for facet_info in sorted_facet_results %}

[simonw]

Really facet_results here should be an array of objects, not an object that maps poorly designed string keys to those objects.

[simonw]

Decision: as an initial fix I'm going to de-duplicate those keys by using tags__array etc - with a _2 on the end if that key is already used.

I'll open a separate issue to redesign this better for Datasette 1.0.

[simonw]

Here's where facet_results is built up:

datasette/datasette/views/table.py

Lines 752 to 758 in 992496f

	for facet in facet_instances:
	(
	instance_facet_results,
	instance_facets_timed_out,
	) = await facet.facet_results()
	facet_results.update(instance_facet_results)
	facets_timed_out.extend(instance_facets_timed_out)

So the decision to key things based on column name is actually embedded deep in the existing facet classes here:

datasette/datasette/facets.py

Lines 224 to 226 in 992496f

	facet_results[column] = {
	"name": column,
	"type": self.type,

datasette/datasette/facets.py

Lines 395 to 397 in 992496f

	facet_results[column] = {
	"name": column,
	"type": self.type,

datasette/datasette/facets.py

Lines 510 to 512 in 992496f

	facet_results[column] = {
	"name": column,
	"type": self.type,

[simonw]

This bad design is even covered in the plugin hooks documentation: https://docs.datasette.io/en/0.59.4/plugin_hooks.html#register-facet-classes

It does at least have the following warning:

Warning

The design of this plugin hook is unstable and may change. See issue 830.

[simonw]

Good news - GitHub's new code search doesn't show ANYONE using that plugin hook - not surprising since it has that documentation warning plus it's just not a very clearly usable hook: https://cs.github.com/?scopeName=All+repos&scope=&q=register_facet_classes%20-repo%3Asimonw%2Fdatasette

[simonw]

Since no-one is using that plugin hook I'm going to alter its contract slightly. I'll still keep the existing JSON format working though (until 1.0), since it's much more likely that people are using that JSON somewhere.

[simonw]

I updated the example code in the facet plugin hook documentation: https://github.com/simonw/datasette/blob/95d0dd7a1cf6be6b7da41e1404184217eb93f64a/docs/plugin_hooks.rst#register_facet_classes

[simonw]

Demo of the fix: https://latest.datasette.io/fixtures/facetable?_facet=created&_facet_date=created&_facet=tags&_facet_array=tags#facet-tags

[simonw]

And here's the new JSON: https://latest.datasette.io/fixtures/facetable.json?_facet=created&_facet_date=created&_facet=tags&_facet_array=tags&_nosuggest=1

{
  "database": "fixtures",
  "table": "facetable",
  "is_view": false,
  "human_description_en": "",
  ...
  "facet_results": {
    "created": {
      "name": "created",
      "type": "column",
      ...
    },
    "tags": {
      "name": "tags",
      "type": "column",
      ...
    },
    "created_2": {
      "name": "created",
      "type": "date",
      ...
    },
    "tags_2": {
      "name": "tags",
      "type": "array",
      ...
    }
  }
}