Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Adding missing type and origin_server_ts properties in relations #9173

Closed

Conversation

jerinjtitus
Copy link
Contributor

@jerinjtitus jerinjtitus commented Jan 20, 2021

Signed-off-by: Jerin J Titus [email protected]

Pull Request Checklist

  • Pull request is based on the develop branch
  • Pull request includes a changelog file. The entry should:
    • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
    • Use markdown where necessary, mostly for code blocks.
    • End with either a period (.) or an exclamation mark (!).
    • Start with a capital letter.
  • Pull request includes a sign off
  • Code style is correct (run the linters)

Adding missing type and origin_server_ts properties in m.reference and m.annotation relations repectively. (Issue #7941) (and #7941 (comment))

@clokep
Copy link
Member

clokep commented Jan 20, 2021

@jerinjtitus Thanks for your contribution! Looks like the tests need to get updated for these changes. Please shout if you have any questions!

@jerinjtitus jerinjtitus marked this pull request as ready for review January 20, 2021 15:43
@anoadragon453
Copy link
Member

anoadragon453 commented Apr 9, 2021

Hey @jerinjtitus, are you able to continue working on this PR? If you need help with the tests, let us know!

You can see which unit tests are failing in CI here: https://buildkite.com/matrix-dot-org/synapse/builds/14649#a7752cf7-d23c-40e6-b223-4d3907a74e85/53-2261

Looks like the tests just need to be updated to include the new keys, for instance in test_aggregation:

"chunk": [
{"type": "m.reaction", "key": "a", "count": 2},
{"type": "m.reaction", "key": "b", "count": 1},
]

The logs from the failing SyTest tests also reveal that the proposed SQL isn't quite valid:

2021-01-20 11:57:28,132 - synapse.http.server - 79 - ERROR - GET-1690 - Failed handle request via 'RoomInitialSyncRestServlet': <SynapseRequest at 0x7f6a5bc43730 method='GET' uri='/_matrix/client/r0/rooms/!EYxKLcBTLFEdNTDMpn:localhost:8800/initialSync?access_token=<redacted>' clientproto='HTTP/1.1' site='8800'>
Traceback (most recent call last):
  File "/venv/lib/python3.8/site-packages/synapse/http/server.py", line 224, in _async_render_wrapper
    callback_return = await self._async_render(request)
  File "/venv/lib/python3.8/site-packages/synapse/http/server.py", line 402, in _async_render
    callback_return = await raw_callback_return
  File "/venv/lib/python3.8/site-packages/synapse/rest/client/v1/room.py", line 588, in on_GET
    content = await self.initial_sync_handler.room_initial_sync(
  File "/venv/lib/python3.8/site-packages/synapse/handlers/initial_sync.py", line 292, in room_initial_sync
    result = await self._room_initial_sync_joined(
  File "/venv/lib/python3.8/site-packages/synapse/handlers/initial_sync.py", line 379, in _room_initial_sync_joined
    state = await self._event_serializer.serialize_events(
  File "/venv/lib/python3.8/site-packages/twisted/internet/defer.py", line 1416, in _inlineCallbacks
    result = result.throwExceptionIntoGenerator(g)
  File "/venv/lib/python3.8/site-packages/twisted/python/failure.py", line 512, in throwExceptionIntoGenerator
    return g.throw(self.type, self.value, self.tb)
  File "/venv/lib/python3.8/site-packages/synapse/storage/databases/main/relations.py", line 226, in get_aggregation_groups_for_event
    return await self.db_pool.runInteraction(
  File "/venv/lib/python3.8/site-packages/synapse/storage/database.py", line 656, in runInteraction
    result = await self.runWithConnection(
  File "/venv/lib/python3.8/site-packages/synapse/storage/database.py", line 739, in runWithConnection
    return await make_deferred_yieldable(
  File "/venv/lib/python3.8/site-packages/twisted/python/threadpool.py", line 250, in inContext
    result = inContext.theWork()
  File "/venv/lib/python3.8/site-packages/twisted/python/threadpool.py", line 266, in <lambda>
    inContext.theWork = lambda: context.call(ctx, func, *args, **kw)
  File "/venv/lib/python3.8/site-packages/twisted/python/context.py", line 122, in callWithContext
    return self.currentContext().callWithContext(ctx, func, *args, **kw)
  File "/venv/lib/python3.8/site-packages/twisted/python/context.py", line 85, in callWithContext
    return func(*args,**kw)
  File "/venv/lib/python3.8/site-packages/twisted/enterprise/adbapi.py", line 306, in _runWithConnection
    compat.reraise(excValue, excTraceback)
  File "/venv/lib/python3.8/site-packages/twisted/python/compat.py", line 464, in reraise
    raise exception.with_traceback(traceback)
  File "/venv/lib/python3.8/site-packages/twisted/enterprise/adbapi.py", line 297, in _runWithConnection
    result = func(conn, *args, **kw)
  File "/venv/lib/python3.8/site-packages/synapse/storage/database.py", line 734, in inner_func
    return func(db_conn, *args, **kwargs)
  File "/venv/lib/python3.8/site-packages/synapse/storage/database.py", line 534, in new_transaction
    r = func(cursor, *args, **kwargs)
  File "/venv/lib/python3.8/site-packages/synapse/storage/databases/main/relations.py", line 204, in _get_aggregation_groups_for_event_txn
    txn.execute(sql, where_args + [limit + 1])
  File "/venv/lib/python3.8/site-packages/synapse/storage/database.py", line 288, in execute
    self._do_execute(self.txn.execute, sql, *args)
  File "/venv/lib/python3.8/site-packages/synapse/storage/database.py", line 314, in _do_execute
    return func(sql, *args)
psycopg2.errors.GroupingError: column "events.origin_server_ts" must appear in the GROUP BY clause or be used in an aggregate function
LINE 1: ...ey, COUNT(DISTINCT sender), MAX(stream_ordering), origin_ser...

@erikjohnston erikjohnston added the X-Awaiting-Changes A contributed PR which needs changes and re-review before it can be merged label May 20, 2021
@richvdh
Copy link
Member

richvdh commented Jun 2, 2021

looks like this is abandoned, so I'm going to close it.

@richvdh richvdh closed this Jun 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
X-Awaiting-Changes A contributed PR which needs changes and re-review before it can be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants