Archives/gristlabs_grist-core
1
0
Fork 0
mirror of https://github.com/gristlabs/grist-core.git synced 2024-10-27 20:44:07 +00:00
Code Issues Projects Releases Wiki Activity

(core) Fix issue with spurious changes produced by Calculate action.

Browse Source 
Summary:
- Replace unicode strings with byte strings when decoding values in sandbox.
- Columns that rely on float values should derive from NumericColumn, so
  that set() ensures that a float is stored even if loading an int.
- Parse unmarshallable values (['U']) into an object that can be encoded
  back to the same value (rather than info a RaisedException).
- Compare NaN's as equal for deciding whether a change is a no-op.

Unrelated:
- Removed a tiny bit of unhelpful logging

Test Plan:
Added a test case that reproduces several causes of Calculate
discrepancies by loading various values into various types of formula columns.

Reviewers: paulfitz

Reviewed By: paulfitz

Differential Revision: https://phab.getgrist.com/D2676
This commit is contained in:
Dmitry S 2020-12-01 16:53:33 -05:00
parent 0e2deecc55
commit 0289e3ea17
4 changed files with 42 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
app/server/lib/ActiveDoc.ts
View File

@ -564,7 +564,6 @@ export class ActiveDoc extends EventEmitter {
*/
public async fetchTable(docSession: OptDocSession, tableId: string,
waitForFormulas: boolean = false): Promise<TableDataAction> {
this.logInfo(docSession, "fetchTable(%s, %s)", docSession, tableId);
return this.fetchQuery(docSession, {tableId, filters: {}}, waitForFormulas);
}
@ -593,7 +592,7 @@ export class ActiveDoc extends EventEmitter {
const wantFull = waitForFormulas || query.tableId.startsWith('_grist_') ||
tableAccess.read === 'mixed';
const onDemand = this._onDemandActions.isOnDemand(query.tableId);
this.logInfo(docSession, "fetchQuery(%s, %s) %s", docSession, JSON.stringify(query),
this.logInfo(docSession, "fetchQuery %s %s", JSON.stringify(query),
onDemand ? "(onDemand)" : "(regular)");
let data: TableDataAction;
if (onDemand) {

6
sandbox/grist/column.py
View File

@ -239,7 +239,7 @@ class NumericColumn(BaseColumn):
_sample_date = moment.ts_to_date(0)
_sample_datetime = moment.ts_to_dt(0, None, moment.TZ_UTC)
class DateColumn(BaseColumn):
class DateColumn(NumericColumn):
"""
DateColumn contains numerical timestamps represented as seconds since epoch, in type float,
to midnight of specific UTC dates. Accessing them yields date objects.
@ -250,7 +250,7 @@ class DateColumn(BaseColumn):
def sample_value(self):
return _sample_date
class DateTimeColumn(BaseColumn):
class DateTimeColumn(NumericColumn):
"""
DateTimeColumn contains numerical timestamps represented as seconds since epoch, in type float,
and a timestamp associated with the column. Accessing them yields datetime objects.
@ -265,7 +265,7 @@ class DateTimeColumn(BaseColumn):
def sample_value(self):
return _sample_datetime
class PositionColumn(BaseColumn):
class PositionColumn(NumericColumn):
def __init__(self, table, col_id, col_info):
super(PositionColumn, self).__init__(table, col_id, col_info)
# This is a list of row_ids, ordered by the position.

26
sandbox/grist/objtypes.py
View File

@ -14,6 +14,7 @@ of the form ['U', repr(obj)].
import exceptions
import traceback
from datetime import date, datetime
from math import isnan
import moment
import records
@ -92,6 +93,14 @@ class AltText(object):
raise InvalidTypedValue(self._typename, self._text)
class UnmarshallableValue(object):
"""
Represents an UnmarshallableValue. There is nothing we can do with it except encode it back.
"""
def __init__(self, value_repr):
self.value_repr = value_repr
# Unique sentinel value representing a pending value. It's encoded as ['P'], and shown to the user
# as "Loading..." text. With the switch to stored formulas, it's currently only used when a
# document was just migrated.
@ -122,9 +131,11 @@ def strict_equal(a, b):
return False
def equal_encoding(a, b):
if isinstance(a, (str, unicode, float, bool, long, int)) or a is None:
# pylint: disable=unidiomatic-typecheck
if isinstance(a, (str, unicode, bool, long, int)) or a is None:
return type(a) == type(b) and a == b
if isinstance(a, float):
return type(a) == type(b) and (a == b or (isnan(a) and isnan(b)))
return encode_object(a) == encode_object(b)
def encode_object(value):
@ -160,6 +171,8 @@ def encode_object(value):
return ['O', {key: encode_object(val) for key, val in value.iteritems()}]
elif value == _pending_sentinel:
return ['P']
elif isinstance(value, UnmarshallableValue):
return ['U', value.value_repr]
except Exception as e:
pass
# We either don't know how to convert the value, or failed during the conversion. Instead we
@ -174,6 +187,13 @@ def decode_object(value):
"""
try:
if not isinstance(value, (list, tuple)):
if isinstance(value, unicode):
# TODO For now, the sandbox uses binary strings throughout; see TODO in main.py for more
# on this. Strings that come from JS become Python binary strings, and we will not see
# unicode here. But we may see it if unmarshalling data that comes from DB, since
# DocStorage encodes/decodes values by marshaling JS strings as unicode. For consistency,
# convert those unicode strings to binary strings too.
return value.encode('utf8')
return value
code = value[0]
args = value[1:]
@ -188,9 +208,11 @@ def decode_object(value):
elif code == 'L':
return [decode_object(item) for item in args]
elif code == 'O':
return {key: decode_object(val) for key, val in args[0].iteritems()}
return {decode_object(key): decode_object(val) for key, val in args[0].iteritems()}
elif code == 'P':
return _pending_sentinel
elif code == 'U':
return UnmarshallableValue(args[0])
raise KeyError("Unknown object type code %r" % code)
except Exception as e:
return RaisedException(e)

25
sandbox/grist/test_types.py
View File

@ -169,12 +169,12 @@ class TestTypes(test_engine.EngineTestCase):
"stored": [
["ModifyColumn", "Types", "date", {"type": "Text"}],
["BulkUpdateRecord", "Types", [13, 14, 15, 16, 17, 18],
{"date": ["False", "True", "1509556595", "8.153", "0", "1"]}],
{"date": ["False", "True", "1509556595.0", "8.153", "0.0", "1.0"]}],
["UpdateRecord", "_grist_Tables_column", 25, {"type": "Text"}]
],
"undo": [
["BulkUpdateRecord", "Types", [13, 14, 15, 16, 17, 18],
{"date": [False, True, 1509556595, 8.153, 0, 1]}],
{"date": [False, True, 1509556595.0, 8.153, 0.0, 1.0]}],
["ModifyColumn", "Types", "date", {"type": "Date"}],
["UpdateRecord", "_grist_Tables_column", 25, {"type": "Date"}]
]
@ -187,10 +187,10 @@ class TestTypes(test_engine.EngineTestCase):
[12, "Chîcágö", "Chîcágö", "Chîcágö", "Chîcágö", "Chîcágö"],
[13, False, "False", "False", "False", "False"],
[14, True, "True", "True", "True", "True"],
[15, 1509556595, "1509556595.0","1509556595","1509556595","1509556595"],
[15, 1509556595, "1509556595.0","1509556595","1509556595","1509556595.0"],
[16, 8.153, "8.153", "8.153", "8.153", "8.153"],
[17, 0, "0.0", "0", "False", "0"],
[18, 1, "1.0", "1", "True", "1"],
[17, 0, "0.0", "0", "False", "0.0"],
[18, 1, "1.0", "1", "True", "1.0"],
[19, "", "", "", "", ""],
[20, None, None, None, None, None]
])
@ -267,13 +267,13 @@ class TestTypes(test_engine.EngineTestCase):
self.assertPartialOutActions(out_actions, {
"stored": [
["ModifyColumn", "Types", "date", {"type": "Numeric"}],
["BulkUpdateRecord", "Types", [13, 14, 15, 17, 18, 19],
{"date": [0.0, 1.0, 1509556595.0, 0.0, 1.0, None]}],
["BulkUpdateRecord", "Types", [13, 14, 19],
{"date": [0.0, 1.0, None]}],
["UpdateRecord", "_grist_Tables_column", 25, {"type": "Numeric"}]
],
"undo": [
["BulkUpdateRecord", "Types", [13, 14, 15, 17, 18, 19],
{"date": [False, True, 1509556595, 0, 1, ""]}],
["BulkUpdateRecord", "Types", [13, 14, 19],
{"date": [False, True, ""]}],
["ModifyColumn", "Types", "date", {"type": "Date"}],
["UpdateRecord", "_grist_Tables_column", 25, {"type": "Date"}]
]
@ -367,12 +367,13 @@ class TestTypes(test_engine.EngineTestCase):
self.assertPartialOutActions(out_actions, {
"stored": [
["ModifyColumn", "Types", "date", {"type": "Int"}],
["BulkUpdateRecord", "Types", [13, 14, 16, 19], {"date": [0, 1, 8, None]}],
["BulkUpdateRecord", "Types", [13, 14, 15, 16, 17, 18, 19],
{"date": [0, 1, 1509556595, 8, 0, 1, None]}],
["UpdateRecord", "_grist_Tables_column", 25, {"type": "Int"}]
],
"undo": [
["BulkUpdateRecord", "Types", [13, 14, 16, 19],
{"date": [False, True, 8.153, ""]}],
["BulkUpdateRecord", "Types", [13, 14, 15, 16, 17, 18, 19],
{"date": [False, True, 1509556595.0, 8.153, 0.0, 1.0, ""]}],
["ModifyColumn", "Types", "date", {"type": "Date"}],
["UpdateRecord", "_grist_Tables_column", 25, {"type": "Date"}]
]