fix(Group Node): rename `time` field to `ctime` for consistency with other node types

[superstar54]

I tried to query some group with a ctime filter, but got this error:

ValueError: ctime is not a column of aliased(DbGroup)
Valid columns are:
id
uuid
label
type_string
time
description
extras
user_id

the time should be ctime for consistency with other node types.

[codecov[bot]]

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 78.31%. Comparing base (660fec7) to head (344de99). :warning: Report is 71 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #6828   +/-   ##
=======================================
  Coverage   78.31%   78.31%           
=======================================
  Files         566      566           
  Lines       42762    42762           
=======================================
  Hits        33484    33484           
  Misses       9278     9278           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[khsrali]

Thanks a lot @superstar54 for pointing this out, Is this change backward compatible? I mean if a group was previously created with time field, would it be queryable now via ctime?

[superstar54]

Thanks for looking into this PR. Upon further inspection, I realized that updating only the fields is not sufficient to fully rename the time field to ctime. We also need to update the backend model and database schema accordingly.

Is this change backward compatible?

No. Users will need to migrate their database to the new schema to reflect the new column name (ctime).

I would say this is beyond my current knowledge, it would be good for someone else in the team, who knows better backend and migration, to take over or advise on the correct implementation.

[superstar54]

Given the complexity of the required schema changes and the time needed to investigate and implement them, I’ve removed it from the v2.7.0 project.

[khsrali]

Hi @superstar54

No. Users will need to migrate their database to the new schema to reflect the new column name (ctime).

Or alternatively, we can mark time deprecated, and query for both time and ctime under the hood. I'd say it's the best to resolve it in a way that we ask users to do nothing.

[superstar54]

we can mark time deprecated, and query for both time and ctime under the hood.

I am not sure if this is possible. Because the ctime column does not exist in the group table in the database created previously, attempting to query or project the field may raise an error.

[edan-bainglass]

@superstar54 so this is not a typo after all, correct? It is time in the DB schema?

[GeigerJ2]

@edan-bainglass, unfortunately, it is 🥲 https://github.com/aiidateam/aiida-core/blob/5e4da5b4dfd00b6ae270e020b8446887b1754230/src/aiida/storage/psql_dos/models/group.py#L41-L58

Also for the DbLog and DbSetting tables, btw. So, updating it to ctime everywhere would require a database migration :/ I get the original motivation to keep it simple if no two time-related entries are being stored, however, indeed, it makes code snippets for one type not transferrable to other types. Would be nice to get a quick hack around this, as @superstar54 suggested, however, "there is nothing as permanent as temporary fixes"...