Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Records grouping bugfix #3751

Merged
merged 6 commits into from
Aug 13, 2024
+125 −55
Merged

Records grouping bugfix #3751

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
eb0df8d
add aggregation step to collect result indices
mathemancer Aug 12, 2024
8ac46c7
update documentation to represent new behavior
mathemancer Aug 12, 2024
a32d79c
use row_number from the result set for result_indices
mathemancer Aug 13, 2024
27be75c
Merge branch 'develop' into records_grouping_bugfix
mathemancer Aug 13, 2024
6910525
fix documentation to align with array indices rather than pkeys
mathemancer Aug 13, 2024
db07f03
Merge branch 'develop' into records_grouping_bugfix
mathemancer Aug 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 53 additions & 25 deletions db/sql/00_msar.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3502,13 +3502,16 @@ $$ LANGUAGE SQL STABLE;


CREATE OR REPLACE FUNCTION
msar.build_results_jsonb_expr(tab_id oid, cte_name text) RETURNS TEXT AS $$/*
msar.build_results_jsonb_expr(tab_id oid, cte_name text, order_ jsonb) RETURNS TEXT AS $$/*
Build an SQL expresson string that, when added to the record listing query, produces a JSON array
with the records resulting from the request.
*/
SELECT 'coalesce(jsonb_agg(json_build_object('
SELECT format(
'coalesce(jsonb_agg(json_build_object('
|| string_agg(format('%1$L, %2$I.%1$I', attnum, cte_name), ', ')
|| ')), jsonb_build_array())'
|| ') %1$s), jsonb_build_array())',
msar.build_order_by_expr(tab_id, order_)
)
FROM pg_catalog.pg_attribute
WHERE
attrelid = tab_id
Expand All @@ -3519,23 +3522,16 @@ $$ LANGUAGE SQL STABLE;


CREATE OR REPLACE FUNCTION
msar.build_grouping_results_jsonb_expr(tab_id oid, cte_name text, group_ jsonb) RETURNS TEXT AS $$/*
Build an SQL expresson string that, when added to the record listing query, produces a JSON array
with the groups resulting from the request.
msar.build_groups_cte_expr(tab_id oid, cte_name text, group_ jsonb) RETURNS TEXT AS $$/*
*/
SELECT format(
$gj$
jsonb_build_object(
'columns', %3$L::jsonb,
'preproc', %4$L::jsonb,
'groups', jsonb_agg(
DISTINCT jsonb_build_object(
'id', %2$I.__mathesar_gid,
'count', %2$I.__mathesar_gcount,
'results_eq', jsonb_build_object(%1$s)
)
)
)
__mathesar_gid AS id,
__mathesar_gcount AS count,
jsonb_build_object(%1$s) AS results_eq,
jsonb_agg(__mathesar_result_idx) AS result_indices
FROM %2$I
GROUP BY id, count, results_eq
$gj$,
string_agg(
format(
Expand All @@ -3548,9 +3544,7 @@ SELECT format(
),
', ' ORDER BY ordinality
),
cte_name,
group_ ->> 'columns',
group_ ->> 'preproc'
cte_name
)
FROM msar.expr_templates RIGHT JOIN ROWS FROM(
jsonb_array_elements_text(group_ -> 'columns'),
Expand All @@ -3560,6 +3554,33 @@ WHERE has_column_privilege(tab_id, col_id::smallint, 'SELECT');
$$ LANGUAGE SQL STABLE RETURNS NULL ON NULL INPUT;


CREATE OR REPLACE FUNCTION
msar.build_grouping_results_jsonb_expr(tab_id oid, cte_name text, group_ jsonb) RETURNS TEXT AS $$/*
Build an SQL expresson string that, when added to the record listing query, produces a JSON array
with the groups resulting from the request.
*/
SELECT format(
$gj$
jsonb_build_object(
'columns', %2$L::jsonb,
'preproc', %3$L::jsonb,
'groups', jsonb_agg(
DISTINCT jsonb_build_object(
'id', %1$I.id,
'count', %1$I.count,
'results_eq', %1$I.results_eq,
'result_indices', %1$I.result_indices
)
)
)
$gj$,
cte_name,
group_ ->> 'columns',
group_ ->> 'preproc'
)
$$ LANGUAGE SQL STABLE RETURNS NULL ON NULL INPUT;


CREATE OR REPLACE FUNCTION
msar.build_selectable_column_expr(tab_id oid) RETURNS text AS $$/*
Build an SQL select-target expression of only columns to which the user has access.
Expand Down Expand Up @@ -3613,14 +3634,20 @@ BEGIN
SELECT count(1) AS count FROM %2$I.%3$I %7$s
), enriched_results_cte AS (
SELECT %1$s, %8$s FROM %2$I.%3$I %7$s %6$s LIMIT %4$L OFFSET %5$L
), results_ranked_cte AS (
SELECT *, row_number() OVER (%6$s) - 1 AS __mathesar_result_idx FROM enriched_results_cte
), groups_cte AS (
SELECT %11$s
)
SELECT jsonb_build_object(
'results', %9$s,
'count', coalesce(max(count_cte.count), 0),
'grouping', %10$s,
'query', $iq$SELECT %1$s FROM %2$I.%3$I %7$s %6$s LIMIT %4$L OFFSET %5$L$iq$
)
FROM enriched_results_cte, count_cte
FROM enriched_results_cte
LEFT JOIN groups_cte ON enriched_results_cte.__mathesar_gid = groups_cte.id
CROSS JOIN count_cte
$q$,
msar.build_selectable_column_expr(tab_id),
msar.get_relation_schema_name(tab_id),
Expand All @@ -3630,8 +3657,9 @@ BEGIN
msar.build_order_by_expr(tab_id, order_),
msar.build_where_clause(tab_id, filter_),
msar.build_grouping_expr(tab_id, group_),
msar.build_results_jsonb_expr(tab_id, 'enriched_results_cte'),
COALESCE(msar.build_grouping_results_jsonb_expr(tab_id, 'enriched_results_cte', group_), 'NULL')
msar.build_results_jsonb_expr(tab_id, 'enriched_results_cte', order_),
COALESCE(msar.build_grouping_results_jsonb_expr(tab_id, 'groups_cte', group_), 'NULL'),
COALESCE(msar.build_groups_cte_expr(tab_id, 'results_ranked_cte', group_), 'NULL AS id')
) INTO records;
RETURN records;
END;
Expand Down Expand Up @@ -3816,7 +3844,7 @@ BEGIN
$i$,
msar.build_single_insert_expr(tab_id, rec_def),
msar.build_selectable_column_expr(tab_id),
msar.build_results_jsonb_expr(tab_id, 'insert_cte')
msar.build_results_jsonb_expr(tab_id, 'insert_cte', null)
) INTO rec_created;
RETURN rec_created;
END;
Expand Down Expand Up @@ -3870,7 +3898,7 @@ BEGIN
)
),
msar.build_selectable_column_expr(tab_id),
msar.build_results_jsonb_expr(tab_id, 'update_cte')
msar.build_results_jsonb_expr(tab_id, 'update_cte', null)
) INTO rec_modified;
RETURN rec_modified;
END;
Expand Down
92 changes: 62 additions & 30 deletions db/sql/test_00_msar.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2901,27 +2901,27 @@ CREATE TABLE "Customers" (
);
ALTER TABLE "Customers" DROP COLUMN "dropmeee 1";
INSERT INTO "Customers" ("First Name", "Last Name", "Subscription Date") VALUES
('Aaron', 'Adams', '2020-03-21'),
('Abigail', 'Acosta', '2020-04-16'),
('Aaron', 'Adams', '2020-04-29'),
('Abigail', 'Adams', '2020-05-29'),
('Abigail', 'Abbott', '2020-07-05'),
('Aaron', 'Adkins', '2020-08-16'),
('Aaron', 'Acevedo', '2020-10-29'),
('Abigail', 'Abbott', '2020-10-30'),
('Abigail', 'Adams', '2021-02-14'),
('Abigail', 'Acevedo', '2021-03-29'),
('Aaron', 'Acosta', '2021-04-13'),
('Aaron', 'Adams', '2021-06-30'),
('Abigail', 'Adkins', '2021-09-12'),
('Aaron', 'Adams', '2021-11-11'),
('Abigail', 'Abbott', '2021-11-30'),
('Aaron', 'Acevedo', '2022-02-04'),
('Aaron', 'Adkins', '2022-03-10'),
('Abigail', 'Abbott', '2022-03-23'),
('Abigail', 'Adkins', '2022-03-27'),
('Abigail', 'Abbott', '2022-04-29'),
('Abigail', 'Adams', '2022-05-24');
('Aaron', 'Adams', '2020-03-21'), -- 1
('Abigail', 'Acosta', '2020-04-16'), -- 2
('Aaron', 'Adams', '2020-04-29'), -- 3
('Abigail', 'Adams', '2020-05-29'), -- 4
('Abigail', 'Abbott', '2020-07-05'), -- 5
('Aaron', 'Adkins', '2020-08-16'), -- 6
('Aaron', 'Acevedo', '2020-10-29'), -- 7
('Abigail', 'Abbott', '2020-10-30'), -- 8
('Abigail', 'Adams', '2021-02-14'), -- 9
('Abigail', 'Acevedo', '2021-03-29'), -- 10
('Aaron', 'Acosta', '2021-04-13'), -- 11
('Aaron', 'Adams', '2021-06-30'), -- 12
('Abigail', 'Adkins', '2021-09-12'), -- 13
('Aaron', 'Adams', '2021-11-11'), -- 14
('Abigail', 'Abbott', '2021-11-30'), -- 15
('Aaron', 'Acevedo', '2022-02-04'), -- 16
('Aaron', 'Adkins', '2022-03-10'), -- 17
('Abigail', 'Abbott', '2022-03-23'), -- 18
('Abigail', 'Adkins', '2022-03-27'), -- 19
('Abigail', 'Abbott', '2022-04-29'), -- 20
('Abigail', 'Adams', '2022-05-24'); -- 21
END;
$$ LANGUAGE plpgsql;

Expand Down Expand Up @@ -3100,11 +3100,36 @@ BEGIN
"columns": [3, 2],
"preproc": null,
"groups": [
{"id": 1, "count": 5, "results_eq": {"2": "Abigail", "3": "Abbott"}},
{"id": 2, "count": 2, "results_eq": {"2": "Aaron", "3": "Acevedo"}},
{"id": 3, "count": 1, "results_eq": {"2": "Abigail", "3": "Acevedo"}},
{"id": 4, "count": 1, "results_eq": {"2": "Aaron", "3": "Acosta"}},
{"id": 5, "count": 1, "results_eq": {"2": "Abigail", "3": "Acosta"}}
{
"id": 1,
"count": 5,
"results_eq": {"2": "Abigail", "3": "Abbott"},
"result_indices": [0, 1, 2, 3, 4]
},
{
"id": 2,
"count": 2,
"results_eq": {"2": "Aaron", "3": "Acevedo"},
"result_indices": [5, 6]
},
{
"id": 3,
"count": 1,
"results_eq": {"2": "Abigail", "3": "Acevedo"},
"result_indices": [7]
},
{
"id": 4,
"count": 1,
"results_eq": {"2": "Aaron", "3": "Acosta"},
"result_indices": [8]
},
{
"id": 5,
"count": 1,
"results_eq": {"2": "Abigail", "3": "Acosta"},
"result_indices": [9]
}
]
}
}$j$ || jsonb_build_object(
Expand Down Expand Up @@ -3135,7 +3160,12 @@ BEGIN
"columns": [3, 2],
"preproc": null,
"groups": [
{"id": 1, "count": 5, "results_eq": {"2": "Abigail", "3": "Abbott"}}
{
"id": 1,
"count": 5,
"results_eq": {"2": "Abigail", "3": "Abbott"},
"result_indices": [0, 1, 2]
}
]
}
}$j$ || jsonb_build_object(
Expand Down Expand Up @@ -3166,8 +3196,8 @@ BEGIN
"columns": [4],
"preproc": ["truncate_to_month"],
"groups": [
{"id": 1, "count": 1, "results_eq": {"4": "2020-03 AD"}},
{"id": 2, "count": 2, "results_eq": {"4": "2020-04 AD"}}
{"id": 1, "count": 1, "results_eq": {"4": "2020-03 AD"}, "result_indices": [0]},
{"id": 2, "count": 2, "results_eq": {"4": "2020-04 AD"}, "result_indices": [1, 2]}
]
}
}$j$ || jsonb_build_object(
Expand Down Expand Up @@ -3199,7 +3229,9 @@ BEGIN
"grouping": {
"columns": [4],
"preproc": ["truncate_to_year"],
"groups": [{"id": 1, "count": 8, "results_eq": {"4": "2020 AD"}}]
"groups": [
{"id": 1, "count": 8, "results_eq": {"4": "2020 AD"}, "result_indices": [0, 1, 2, 3, 4]}
]
}
}$j$ || jsonb_build_object(
'query', concat(
Expand Down
9 changes: 9 additions & 0 deletions mathesar/rpc/records.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ class Grouping(TypedDict):
"""
Grouping definition.

The table involved must have a single column primary key.

Attributes:
columns: The columns to be grouped by.
preproc: The preprocessing funtions to apply (if any).
Expand All @@ -93,14 +95,21 @@ class Group(TypedDict):
"""
Group definition.

Note that the `count` is over all rows in the group, whether returned
or not. However, `result_indices` is restricted to only the rows
returned. This is to avoid potential problems if there are many rows
in the group (e.g., the whole table), but we only return a few.

Attributes:
id: The id of the group. Consistent for same input.
count: The number of items in the group.
results_eq: The value the results of the group equal.
result_indices: The primary key values of group members.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"primary key values" is not accurate here.

"""
id: int
count: int
results_eq: list[dict]
result_indices: list[Any]


class GroupingResponse(TypedDict):
Expand Down
Loading