Skip to content

Commit

Permalink
Merge #1122
Browse files Browse the repository at this point in the history
1122: Proposal: Add `path` column to categories table as `ltree` type to make tree traversing easier r=carols10cents a=erewok

## Description

This is a proposal inspired by tickets ([1](#1093), and [2](#721)) with the goal of eventually revealing more information about categories on pages where category information is available. It is also motivated by the current application code surrounding categories, which contains lots of SQL with operations like this: `split_part(c2.slug, '::', 1)`.

This PR is intended as a suggestion for what to do with the `categories` table.

Following are changes included here:

- Adds a `path` column as a Postgresql `ltree` type to the `categories` table in order to more easily query trees of categories and subcategories. This allows navigating trees of arbitrary depth and potentially more ergonomic path-finding queries.

- Adds a `parent_categories` method to `Category`, which returns parent categories _in order of traversal_ from root to this node.

## Trade-offs to this Approach

### Pros

- Easily handle trees of arbitrary depth, ex: "category::sub-category::sub-catgeory-A::sub-category-A-1".
- Makes queries to find a whole tree of nodes simpler (and possibly quicker?). Ex:
  `select slug, path from categories where path <@ 'root.web_programming';`
- Makes finding all root-level nodes simpler (assuming one is cool with the syntax). Ex:
  `select slug, path from categories where path ~ 'root.*{1}';`
- Makes finding the direct path from the root node to a particular node easier (see `parent_categories` method). Ex:
  `select slug, path from categories where path @> 'root.web_programming.http_client' order by path;`
- Invisible to current application code: nothing needs to be updated to handle the `path` column because it happens via database triggers. (This could also be a _con_ depending on one's perspective.)

### Cons

- Postgresql only (harder to switch databases in the future)
- Makes certain characters in slugs disallowed: `[',",-]`. Path labels must consist of "A-Za-z0-9_" separated by "." We swap `-` for `_` and `::` for `.` in the included Postgresql procedure.
- This `path` column is nowhere in application code, so this is all hand-written SQL (but the current solution is doing that as well).
- Error messages are opaque if you happen to have a bad character in a slug:
```
cargo_registry=# update categories set slug = 'test-new::bla$bla-::parent::child' where slug = 'test-new::parent::child';
ERROR:  syntax error at position 17
CONTEXT:  PL/pgSQL function set_category_path_to_slug() line 3 at assignment
```

## Notes

- [x] Needs a test for the `parent_categories` method.

Co-authored-by: Erik Aker <eraker@gmail.com>
Co-authored-by: Ashe Connor <ashe@kivikakk.ee>
Co-authored-by: Carol (Nichols || Goulding) <carol.nichols@gmail.com>
  • Loading branch information
4 people committed Aug 30, 2018
2 parents 53b8afe + 81dff1e commit f58ca05
Show file tree
Hide file tree
Showing 21 changed files with 667 additions and 85 deletions.
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/target
.cargo
.vscode/

# compiled output
/dist
Expand All @@ -21,3 +22,5 @@ yarn-error.log
testem.log
.env
docker-compose.override.yml
*~

52 changes: 52 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ dotenv = "0.11.0"
toml = "0.4"
diesel = { version = "1.3.0", features = ["postgres", "serde_json", "chrono", "r2d2"] }
diesel_full_text_search = "1.0.0"
diesel_ltree = "0.1.3"
serde_json = "1.0.0"
serde_derive = "1.0.0"
serde = "1.0.0"
Expand Down
1 change: 1 addition & 0 deletions app/models/category.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export default DS.Model.extend({
crates_cnt: DS.attr('number'),

subcategories: DS.attr(),
parent_categories: DS.attr(),

crates: DS.hasMany('crate', { async: true }),
});
7 changes: 5 additions & 2 deletions app/templates/category/index.hbs
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
{{ title category.category ' - Categories' }}

<div id='crates-heading'>
{{svg-jar "crate"}}
<h1>{{ category.category }}</h1>
{{#link-to "categories" (html-attributes aria-label="Categories")}}{{svg-jar "crate"}}{{/link-to}}
<h1>
{{#each category.parent_categories as |parent|}}{{link-to parent.category "category" parent.slug}}::{{/each}}
{{~ category.category }}
</h1>
</div>

<div>
Expand Down
2 changes: 1 addition & 1 deletion diesel.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@
[print_schema]
file = "src/schema.rs"
with_docs = true
import_types = ["diesel::sql_types::*", "diesel_full_text_search::{TsVector as Tsvector}"]
import_types = ["diesel::sql_types::*", "diesel_full_text_search::{TsVector as Tsvector}", "diesel_ltree::Ltree"]
patch_file = "src/schema.patch"
14 changes: 14 additions & 0 deletions migrations/2017-10-08-193512_category_trees/down.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
-- This file should undo anything in `up.sql`
DROP TRIGGER set_category_path_update ON categories;

DROP TRIGGER set_category_path_insert ON categories;

DROP FUNCTION IF EXISTS set_category_path_to_slug();

DROP INDEX path_categories_idx;
DROP INDEX path_gist_categories_idx;

ALTER TABLE categories
DROP COLUMN path;

DROP EXTENSION ltree;
40 changes: 40 additions & 0 deletions migrations/2017-10-08-193512_category_trees/up.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
-- Your SQL goes here
CREATE EXTENSION ltree;

-- Create the new column which will represent our category tree.
-- Fill it with values from `slug` column and then set to non-null
ALTER TABLE categories
ADD COLUMN path ltree;

-- Unfortunately, hyphens (dashes) are not allowed...
UPDATE categories
SET path = text2ltree('root.' || trim(replace(replace(slug, '-', '_'), '::', '.')))
WHERE path is NULL;

ALTER TABLE CATEGORIES
ALTER COLUMN path SET NOT NULL;

-- Create some indices that allow us to use GIST operators: '@>', etc
CREATE INDEX path_gist_categories_idx ON categories USING GIST(path);
CREATE INDEX path_categories_idx ON categories USING btree(path);

-- Create procedure and trigger to auto-update path
CREATE OR REPLACE FUNCTION set_category_path_to_slug()
RETURNS trigger AS
$BODY$
BEGIN
NEW.path = text2ltree('root.' || trim(replace(replace(NEW.slug, '-', '_'), '::', '.')));
RETURN NEW;
END;
$BODY$ LANGUAGE plpgsql;

CREATE TRIGGER set_category_path_insert
BEFORE INSERT
ON categories
FOR EACH ROW
EXECUTE PROCEDURE set_category_path_to_slug();

CREATE TRIGGER set_category_path_update
BEFORE UPDATE OF slug ON categories
FOR EACH ROW
EXECUTE PROCEDURE set_category_path_to_slug();
Loading

0 comments on commit f58ca05

Please sign in to comment.