(core) Return RecordSet instead of list from property access when possible, to allow further property access

Summary: While `$ref.other_ref` returns a reference (Record) allowing chaining more properties like `$ref.other_ref.foo`, reflists (RecordSet) did not allow this, e.g. `$reflist.other_ref` returned a plain list of records, preventing chaining more dot notation. Discussed here: https://grist.slack.com/archives/CDHABLZJT/p1648845745765839

Test Plan: Added a Python unit test. Formulas like `$reflist.other_ref` were already very common though, and getting the functionality code slightly wrong leads to a flood of test failures.

Reviewers: jarek

Reviewed By: jarek

Subscribers: jarek

Differential Revision: https://phab.getgrist.com/D3354
This commit is contained in:
Alex Hall 2022-04-04 21:09:48 +02:00
parent fea8f906d7
commit 8ec823e7ee
2 changed files with 41 additions and 4 deletions

View File

@ -598,8 +598,8 @@ class Table(object):
# Called when record.foo is accessed # Called when record.foo is accessed
def _get_col_value(self, col_id, row_id, relation): def _get_col_value(self, col_id, row_id, relation):
[value] = self._get_col_subset(col_id, [row_id], relation) [value] = self._get_col_subset_raw(col_id, [row_id], relation)
return value return records.adjust_record(relation, value)
def _attribute_error(self, col_id, relation): def _attribute_error(self, col_id, relation):
self._engine._use_node(self._new_columns_node, relation) self._engine._use_node(self._new_columns_node, relation)
@ -607,8 +607,28 @@ class Table(object):
# Called when record_set.foo is accessed # Called when record_set.foo is accessed
def _get_col_subset(self, col_id, row_ids, relation): def _get_col_subset(self, col_id, row_ids, relation):
values = self._get_col_subset_raw(col_id, row_ids, relation)
# When all the values are the same type of Record (i.e. all references to the same table)
# combine them into a single RecordSet for that table instead of a list
# so that more attribute accesses can be chained,
# e.g. record_set.foo.bar where `foo` is a Reference column.
value_types = list(set(map(type, values)))
if len(value_types) == 1 and issubclass(value_types[0], records.Record):
return records.RecordSet(
values[0]._table,
# This is different from row_ids: these are the row IDs referenced by these Records,
# whereas row_ids are where the values were being stored.
[val._row_id for val in values],
relation.compose(values[0]._source_relation),
)
else:
return [records.adjust_record(relation, value) for value in values]
# Internal helper to optimise _get_col_value
# so that it doesn't make a singleton RecordSet just to immediately unpack it
def _get_col_subset_raw(self, col_id, row_ids, relation):
col = self.all_columns[col_id] col = self.all_columns[col_id]
# creates a dependency and brings formula columns up-to-date. # creates a dependency and brings formula columns up-to-date.
self._engine._use_node(col.node, relation, row_ids) self._engine._use_node(col.node, relation, row_ids)
# TODO: when column is a reference, support property access in return value return [col.get_cell_value(row_id) for row_id in row_ids]
return [records.adjust_record(relation, col.get_cell_value(row_id)) for row_id in row_ids]

View File

@ -1,5 +1,7 @@
import testutil import testutil
import test_engine import test_engine
from objtypes import RecordSetStub
class TestRecordList(test_engine.EngineTestCase): class TestRecordList(test_engine.EngineTestCase):
col = testutil.col_schema_row col = testutil.col_schema_row
@ -83,3 +85,18 @@ class TestRecordList(test_engine.EngineTestCase):
[1, "Mammals", [1, 3], [1, 3], True], [1, "Mammals", [1, 3], [1, 3], True],
[2, "Reptilia", [2, 4], [2, 4], True], [2, "Reptilia", [2, 4], [2, 4], True],
]) ])
def test_attribute_chain(self):
self.load_sample(self.sample)
self.add_column('Class', 'Names', type='Any', isFormula=True,
formula="$Creatures.Class.Name")
self.add_column('Class', 'Creatures2', type='Any', isFormula=True,
formula="$Creatures.Class.Creatures")
mammals = RecordSetStub("Creatures", [1, 3])
reptiles = RecordSetStub("Creatures", [2, 4])
self.assertTableData("Class", data=[
["id", "Name", "Creatures", "Names", "Creatures2"],
[1, "Mammals", [1, 3], ["Mammals", "Mammals"], [mammals, mammals]],
[2, "Reptilia", [2, 4], ["Reptilia", "Reptilia"], [reptiles, reptiles]],
])