dbs-leipzig / gradoop

Distributed Temporal Graph Analytics with Apache Flink
https://github.com/dbs-leipzig/gradoop
Apache License 2.0
244 stars 88 forks source link

[#1544] Make temporal grouping result temporal #1554

Closed timo95 closed 1 year ago

timo95 commented 3 years ago

Fixes #1544

Description

Related Issue

1544

Motivation and Context

How Has This Been Tested?

Types of Changes

Checklist:

ChrizZz110 commented 3 years ago

First of all - I'm happy that you took that issue, since I think this is really necessary. According to the realization I have to think about when I read your code ... First thin I noticed is the transaction time, which should not be changed. The transaction times of grouped elements should never changed. tx-from is set, when the super vertex/edge is created by gradoop. I know, this sounds unfamiliar, but this is the semantic of tx-times - they are immutable.

timo95 commented 3 years ago

The transaction time isn't changed in temporalGrouping. I only added the possibility to change it with aggregations. I thought it would be good to have, since you can already read and aggregate it. I can remove this if you want.

ChrizZz110 commented 3 years ago

Any special reason for changing the api of the aggregation class constructors? All systems that use these aggregate classes have to change the signature then

timo95 commented 3 years ago
  1. Keeping the constructor overload prettier (required parameters first, optional ones last)
  2. Following the api of the non-temporal aggregation functions
ChrizZz110 commented 3 years ago

Convinced.

timo95 commented 3 years ago

Done.