Skip to content

fix(etl): count documents, not rows, in joining_props - #319

Open
grzelaka-roche wants to merge 1 commit into
uc-cdis:masterfrom
grzelaka-roche:fix/joining-props-count-documents
Open

grzelaka-roche wants to merge 1 commit into
uc-cdis:masterfrom
grzelaka-roche:fix/joining-props-count-documents

Conversation

@grzelaka-roche

Copy link
Copy Markdown

A joining index is read at its intermediate step, before its own translate_final flattens it to one row per document, so a document linked to several parents is still one row per link at that point. fn: count counted those rows

Count over the key of the joining index instead, and deduplicate the projection on that key so list and sum also see each document once. Both are skipped when the frame does not carry that key, falling back to the previous row count rather than silently merging distinct documents.

Also in the same path:

  • drop the re-join of a key-only frame onto the aggregates, which multiplied every row of the joined index by its document count
  • a group with no matching document now reads 0 instead of null. This is visible in Elasticsearch: guppy sees 0 where the field used to be missing
  • build the aggregation expressions from the props that were actually projected, so a joining_props block mixing props with and without fn no longer builds agg(None)

Link to JIRA ticket if there is one:

New Features

Breaking Changes

Bug Fixes

  • "count" aggregation in joining props now counts actual documents, not just rows and doesn't put "null" but 0 when there is no documents to count

Improvements

Dependency updates

Deployment changes

A joining index is read at its intermediate step, before its own
translate_final flattens it to one row per document, so a document
linked to several parents is still one row per link at that point.
`fn: count` counted those rows: an expression file linked to 23 samples
added 23 to the study's data_file_count instead of 1.

Count over the key of the joining index instead, and deduplicate the
projection on that key so `list` and `sum` also see each document once.
Both are skipped when the frame does not carry that key, falling back to
the previous row count rather than silently merging distinct documents.

Also in the same path:
- drop the re-join of a key-only frame onto the aggregates, which
  multiplied every row of the joined index by its document count
- a group with no matching document now reads 0 instead of null. This
  is visible in Elasticsearch: guppy sees 0 where the field used to be
  missing
- build the aggregation expressions from the props that were actually
  projected, so a joining_props block mixing props with and without
  `fn` no longer builds agg(None)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant