Skip to content

Write to LinkedEdge table in linked_edge_generator for containedInPlace - #769

Open
n-h-diaz wants to merge 2 commits into
datacommonsorg:masterfrom
n-h-diaz:linked-edge-5
Open

n-h-diaz wants to merge 2 commits into
datacommonsorg:masterfrom
n-h-diaz:linked-edge-5

Conversation

@n-h-diaz

Copy link
Copy Markdown
Contributor

This PR updates the LinkedEdgeGenerator.run_linked_contained_in_place to also write to the LinkedEdge table

  • Since the query already computes the recursive relationships, we can write (almost) directly (with also fetching child types)
  • Note: Since we currently only attribute a single provenance, this will use the containment provenance (not the type provenance, which can be anything). This is consistent with the current "filter" behavior (ie. property=<-containedInPlace+{typeOf:<child_type>})

Also

  • Updates a few other linked_edge_generator tests which were failing
  • Updates BigQueryExecutor to use threading.local() so each thread gets its own isolated client for concurrent aggregations

Schema migration changes are in: datacommonsorg/datacommons#275 , but I double check on sequencing for submitting this

Mixer changes are currently flag gated (disabled everywhere)

@codacy-production

codacy-production Bot commented Sep 16, 2026

Copy link
Copy Markdown

Not up to standards ⛔

🔴 Issues 2 medium · 5 minor

Alerts:
⚠ 7 issues (≤ 0 issues of at least minor severity)

Results:
7 new issues

Category Results
Documentation 5 minor
Security 2 medium

View in Codacy

🟢 Metrics 5 complexity

Metric Results
Complexity 5

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces thread-safe lazy initialization of the BigQuery client using threading.local() in bq_executor.py. It also adds a concurrent deletion helper (delete_concurrent) in deleter.py using ThreadPoolExecutor to parallelize Spanner partitioned DML deletions, refactoring existing deletion methods to use it. Additionally, it updates linked_edge_generator.py to export data to both Edge and LinkedEdge tables, along with corresponding updates to tests. Feedback on the changes highlights two main areas of improvement: first, in linked_edge_generator.py, querying all typeOf edges without filtering can lead to a massive full-table scan and duplicate rows, so it is recommended to filter the query to only include subjects of containedInPlace edges and use DISTINCT to prevent primary key violations; second, in deleter.py, if delete_queries is empty, initializing ThreadPoolExecutor with max_workers=0 will raise a ValueError, so an early return guard should be added to handle empty query lists safely.

Comment thread pipeline/workflow/aggregation-helper/aggregation/linked_edge_generator.py Outdated
Comment thread pipeline/workflow/aggregation-helper/aggregation/deleter.py

This branch has not been deployed

No deployments
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.

2 participants