mathesar
mathesar copied to clipboard
mathesar-foundation
Extracting columns that aren't orderable into a different tables results in incorrect data
Description
Extracting columns that aren't orderable (e.g., JSON) into a different table using the move_column or split_table feature results in incorrect data.
Expected behavior
We should be able to move columns that aren't orderable or give a correct error response if it is not possible.
Suggested solution
Detect when the column being moved is not orderable and throw an error.
#1436 needs to be merged for this bug to be valid, marking it as draft until then
For context, we've a predicate function (currently it's private) that tells you if a column is orderable: https://github.com/centerofci/mathesar/blob/73458a90fce21975e87761af32e2a38b59b274e6/db/records/operations/sort.py#L68
@Reireirei0 sure please go ahead.
Since this will be your first contribution please read our contributing guide here: https://wiki.mathesar.org/en/community/contributing
@Reireirei0 to quickly expand on what @rajatvijay said, we're currently not assigning issues to new contributors that don't yet have any PRs (open or closed). This is so that if a contributor were to disappear, that wouldn't block other contributors from picking up the issue he was working on (this is explained in the link @rajatvijay gave).
@dmos62 @silentninja
check my modification, is this the desired solution?
def _is_col_orderable(col):
data_type = col.type
non_orderable_type = ['Binary', 'LargeBinary', 'PickleType', 'ARRAY', 'JSON', 'JSONB']
if str(data_type) in non_orderable_type:
raise ValueError("Column is not orderable.")
return True
@7ashraf no. See https://github.com/centerofci/mathesar/issues/1490#issuecomment-1481147668
@thesujai you'll want to provide tests that make sure the problem is fixed. Please post a PR before asking for review.
@silentninja Where are move_column or split_table function, I want to add try and except logic to this (_is_col_orderable) function.
@rai-ankur You're welcome to work on this and submit a PR, but in general we don't assign tickets to new contributors, as noted in our Contributor Guide. Once we've already merged a PR of yours, then we'll be happy to assign tickets to you after that. The reason for this policy is that in the past it has been a maintenance burden for us to manage ticket assignments while having an overwhelming volume of people fail to submit their first PR when initially expressing intent to contribute.
I've added the db-layer label since the split_tables function is implemented purely in SQL now.
