Skip to content

Consolidate Dataclass data update methods - use DIB for update only - #7483

Merged
XingY merged 39 commits into
developfrom
fb_sourceDIB
May 3, 2026
Merged

Consolidate Dataclass data update methods - use DIB for update only#7483
XingY merged 39 commits into
developfrom
fb_sourceDIB

Conversation

@XingY

@XingYXingY commented Mar 10, 2026

Copy link
Copy Markdown
Contributor

Rationale

This PR consolidates DataClass data update operations to use the DataIteratorBuilder (DIB) pipeline exclusively — the same approach already used for SampleType updates.

Related Pull Requests

Changes

  • DataClass updates now go through the DIB pipeline exclusively.
  • LSID is rejected as a standalone update key with clear error messages.
  • The provisioned table lsid column is dropped via deferred upgrade.
  • Partition-based DIB logic is extracted and shared.
  • getAltKeysForUpdate is removed from the interface and API.

CopilotAI 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.

Pull request overview

This PR consolidates DataClass and Sample update behavior around DataIteratorBuilder (DIB)-based updates, removing LSID as an update/merge key and cleaning up deprecated DataClass storage by dropping the provisioned lsid column via a module upgrade.

Changes:

  • Remove LSID-as-key support for sample/data update & merge; enforce RowId or Name-based keys and ignore LSID when provided alongside valid keys.
  • Introduce shared updateRowsUsingPartitionedDIB() logic in AbstractQueryUpdateService and route Sample/DataClass updates through it.
  • Remove provisioned DataClass lsid column (domain kind + upgrade code + DB scripts) and update integration tests accordingly.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 7 comments.

Show a summary per file
FileDescription
experiment/src/org/labkey/experiment/samples/AbstractExpFolderImporter.javaStops setting the removed UseLsidForUpdate option during TSV import.
experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.javaRoutes updates through partitioned DIB; removes LSID-key paths and legacy update methods.
experiment/src/org/labkey/experiment/api/ExpDataImpl.javaSelects DataClass provisioned properties by RowId (not LSID) to support dropped provisioned LSID column.
experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.javaEnforces RowId/Name keys, blocks LSID-only keying, blocks RowId in merge (behind feature), and uses partitioned DIB updates.
experiment/src/org/labkey/experiment/api/DataClassDomainKind.javaRemoves lsid from provisioned DataClass base properties, indexes, and FKs.
experiment/src/org/labkey/experiment/ExperimentUpgradeCode.javaAdds deferred upgrade to drop provisioned DataClass lsid column (and related indices).
experiment/src/org/labkey/experiment/ExperimentModule.javaBumps schema version to 26.005 and centralizes experimental flag constant usage.
experiment/src/org/labkey/experiment/ExpDataIterators.javaUpdates derivation/update logic to use RowId/Name instead of LSID-for-update config; removes LSID update config usage.
experiment/src/client/test/integration/DataClassCrud.ispec.tsExpands integration coverage for keying behavior (RowId/Name/LSID) and partitioned-duplicate detection.
experiment/resources/schemas/dbscripts/sqlserver/exp-26.004-26.005.sqlExecutes Java upgrade to drop provisioned DataClass lsid column (SQL Server).
experiment/resources/schemas/dbscripts/postgresql/exp-26.004-26.005.sqlExecutes Java upgrade to drop provisioned DataClass lsid column (PostgreSQL).
api/src/org/labkey/api/query/DefaultQueryUpdateService.javaRemoves conversion-table hook and always uses getDbTable() for convertTypes() entrypoint.
api/src/org/labkey/api/query/AbstractQueryUpdateService.javaAdds updateRowsUsingPartitionedDIB() and small null-iterator safety in _pump().
api/src/org/labkey/api/exp/query/ExpDataTable.javaReorders enum constants (no functional behavior change expected).
api/src/org/labkey/api/exp/api/ExperimentService.javaRemoves UseLsidForUpdate query option; adds shared experimental feature constant.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threadexperiment/src/client/test/integration/DataClassCrud.ispec.ts Outdated
Comment threadexperiment/src/client/test/integration/DataClassCrud.ispec.ts Outdated
Comment threadexperiment/src/org/labkey/experiment/ExperimentModule.java Outdated
Comment threadexperiment/src/org/labkey/experiment/ExpDataIterators.java Outdated
Comment threadexperiment/src/org/labkey/experiment/ExpDataIterators.java Outdated
Comment threadexperiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java Outdated
Comment threadexperiment/src/client/test/integration/DataClassCrud.ispec.ts Outdated
@labkey-adam

Copy link
Copy Markdown
Contributor

@XingY there's an exp-26.004-26.005.sql script in 26.3 that should be merged to develop shortly.

@XingY

Copy link
Copy Markdown
ContributorAuthor

@XingY there's an exp-26.004-26.005.sql script in 26.3 that should be merged to develop shortly.

OK. Thanks for the heads-up.

Comment threadapi/src/org/labkey/api/dataiterator/ExistingRecordDataIterator.java Outdated
Comment threadapi/src/org/labkey/api/dataiterator/SampleUpdateAddColumnsDataIterator.java Outdated
Comment threadapi/src/org/labkey/api/exp/xar/LSIDRelativizer.java
Comment threadexperiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java Outdated
Comment threadexperiment/src/org/labkey/experiment/ExpDataIterators.java
Comment threadexperiment/src/org/labkey/experiment/ExperimentModule.java Outdated
@XingY
XingY requested a review from labkey-nickaApril 30, 2026 22:48
@labkey-nicka
labkey-nicka removed the request for review from labkey-matthewbMay 1, 2026 22:51
@XingY
XingY merged commit 5cb6370 into developMay 3, 2026
16 checks passed
@XingY
XingY deleted the fb_sourceDIB branch May 3, 2026 23:32
Sign up for freeto 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.

4 participants

@XingY@labkey-adam@labkey-nicka