Feat: replace sort order #1500

JasperHG90 · 2025-01-08T21:24:41Z

This PR adds functionality to replace a table's sort order. Closes #1245

Some basic tests are implemented but need to be expanded.

Currently, a new sort order ID is assigned. This adds a sort order to the list of available sort orders rather than replacing it.

@Fokko do you mind taking a look at this and telling me what you think? Thanks in advance!

kevinjqliu

Thanks for the PR!
Generally lgtm, added a few nit comments

kevinjqliu · 2025-01-11T19:19:57Z

pyiceberg/table/__init__.py

            name_mapping=self.table_metadata.name_mapping(),
        )

+    def replace_sort_order(self, case_sensitive: bool = True) -> ReplaceSortOrder:


nit: wdyt about update_sort_order instead?

from the spec,

A data or delete file is associated with a sort order by the sort order's id within [a manifest](https://iceberg.apache.org/spec/#manifests). Therefore, the table must declare all the sort orders for lookup. A table could also be configured with a default sort order id, indicating how the new data should be sorted by default. Writers should use this default sort order to sort the data on write, but are not required to if the default order is prohibitively expensive, as it would be for streaming writes.

All sort orders are kept in the manifest in the sort-orders field.

Thanks. Updated class & method names

tests/integration/test_sort_order_update.py

kevinjqliu · 2025-01-11T19:24:25Z

pyiceberg/table/update/sorting.py

+    from pyiceberg.table import Transaction
+
+
+class SortOrderBuilder:


nit: do we need the builder here? Or just follow UpdateSchema's pattern

We don't need it but personally I like it because it's easier to read. I'll leave it up to you to decide if you want this.

Refactored this.

JasperHG90 · 2025-01-16T20:54:09Z

Thanks for the comments 🙌 @kevinjqliu . Will look at your comments this weekend.

…assigned id, else set a previous sort order as the default

JasperHG90 · 2025-03-24T21:50:36Z

pyiceberg/table/update/sorting.py

+    def _reuse_or_create_sort_order_id(self) -> int:
+        """Return the last assigned sort order id or create a new one."""
+        new_sort_order_id = INITIAL_SORT_ORDER_ID
+        for sort_order in self._transaction.table_metadata.sort_orders:
+            new_sort_order_id = max(new_sort_order_id, sort_order.order_id)
+            if sort_order.fields == self._fields:
+                return sort_order.order_id
+            elif new_sort_order_id <= sort_order.order_id:
+                new_sort_order_id = sort_order.order_id + 1
+        return new_sort_order_id


@kevinjqliu added this to reuse an existing sort order instead of creating a new one

JasperHG90 · 2025-03-24T21:50:51Z

pyiceberg/table/update/sorting.py

+        super().__init__(transaction)
+        self._fields: List[SortField] = []
+        self._case_sensitive: bool = case_sensitive
+        self._last_assigned_order_id: Optional[int] = None


@kevinjqliu this is now None by default

…r has been added

JasperHG90 · 2025-03-24T22:00:11Z

pyiceberg/table/update/sorting.py

+        if (
+            self._transaction.table_metadata.default_sort_order_id != new_sort_order.order_id
+            and self._transaction.table_metadata.sort_order_by_id(new_sort_order.order_id) is None
+        ):
+            self._last_assigned_order_id = new_sort_order.order_id
+            updates = (AddSortOrderUpdate(sort_order=new_sort_order), SetDefaultSortOrderUpdate(sort_order_id=-1))


@kevinjqliu last assigned sort order only set if new sort order is created.

JasperHG90 · 2025-04-15T08:07:19Z

@kevinjqliu @Fokko do either one of you have time to review this PR in the coming weeks? I have some time on my hands and I’d love to wrap this PR up! Thanks in advance

Fokko · 2025-05-16T21:23:42Z

Hey @JasperHG90 sorry for the late reply, the notification got buried in my mailbox. I'll definitely review this, it would be great to get this in 👍

Fokko · 2025-05-16T21:27:22Z

pyiceberg/table/__init__.py


+    def update_sort_order(self, case_sensitive: bool = True) -> UpdateSortOrder:
+        """Create a new UpdateSortOrder to update the sort order of this table.
+


nit:

Suggested change

Args:

case_sensitive: If field names are case-sensitive.

JasperHG90 · 2025-05-19T08:36:39Z

Hey @JasperHG90 sorry for the late reply, the notification got buried in my mailbox. I'll definitely review this, it would be great to get this in 👍

No apology necessary. Happy to receive your review!

mwa28 · 2025-07-08T15:08:47Z

Hello, any update on this PR?

Fokko · 2025-08-04T12:04:20Z

@JasperHG90 Sorry for the late reply, and chance that you could revisit this PR?

rambleraptor · 2025-09-16T21:09:05Z

@JasperHG90 are you still working on this? I'm happy to pick it up if you're not

jayceslesar

What if we want to remove the sort order from a table, should there be an explicit API to do so?

Another general question I have is the distinction between modifying an existing sort order and entirely replacing it? Those can technically be expressed the same way but should we make it more obvious to users?

jayceslesar · 2025-09-18T00:27:09Z

pyiceberg/table/update/sorting.py

+    def asc(
+        self, source_column_name: str, transform: Transform[Any, Any], null_order: NullOrder = NullOrder.NULLS_LAST
+    ) -> UpdateSortOrder:
+        """Add a sort field with ascending order."""
+        return self._add_sort_field(
+            source_id=self._column_name_to_id(source_column_name),
+            transform=transform,
+            direction=SortDirection.ASC,
+            null_order=null_order,
+        )
+
+    def desc(
+        self, source_column_name: str, transform: Transform[Any, Any], null_order: NullOrder = NullOrder.NULLS_LAST
+    ) -> UpdateSortOrder:
+        """Add a sort field with descending order."""
+        return self._add_sort_field(
+            source_id=self._column_name_to_id(source_column_name),
+            transform=transform,
+            direction=SortDirection.DESC,
+            null_order=null_order,
+        )


Why are these separate functions instead of letting the user specify via constructing and passing in sort field objects? We already have an API for describing a sort field so why abstract it? If the sort order arguments change now we have to take care of it here too

def _add_sort_field( self, source_id: int, transform: Transform[Any, Any], direction: SortDirection, null_order: NullOrder, ) -> UpdateSortOrder: """Add a sort field to the sort order list.""" self._fields.append( SortField( source_id=source_id, transform=transform, direction=direction, null_order=null_order, ) ) return self

Why do we have this wrapper is my question, can we not acheive the same functionality by letting the user pass in a list of SortField objects, which would mimic the API for creating a table:

sort_order = SortOrder(SortField(source_id=2, transform='identity')) catalog.create_table( identifier="docs_example.bids", schema=schema, partition_spec=partition_spec, sort_order=sort_order, )

jayceslesar · 2025-09-18T00:35:04Z

We could also cover testing against all catalogs via the test_catalog.py in the integration tests!

JasperHG90 · 2025-09-22T13:33:02Z

Hi all. Apologies for my late response. TBH this has slipped from my mind in the past couple of months due to work. @jayceslesar and @rambleraptor thanks for your comments. I have some time on my hands the coming couple of weeks, so I'd be happy to colab on this to finish it up if you like. If you'd like to steamroll ahead and implement it ASAP because you need it, that's also fine by me.

jayceslesar · 2025-09-22T14:03:06Z

Im certainly in no rush so happy to let you see this through! Would be epic to have this in the next minor release (0.11.0) !

@JasperHG90

# Rationale for this change Fixed the tests in #1500, but kept the commits so @JasperHG90 gets all the credits for the great work! Closes #1500 Closes #1245 ## Are these changes tested? ## Are there any user-facing changes?  --------- Co-authored-by: Jasper Ginn <jasperginn@gmail.com>

JasperHG90 added 22 commits January 5, 2025 20:20

chore: scaffolding

ccfcbcd

chore: scaffolding

95da8f3

chore: add skeleton for asc/desc methods

253967d

chore: scaffolding

7b5a98e

chore: change method names

304a806

chore: update methods

48ac5c0

chore: update methods

a47067c

chore: update imports

c1ab2ec

chore: stupid renames

8f27d14

chore: lint

d8720f2

chore: docstrings

90db60a

test: add integration test for replace sort order

1cd2729

test: add test for lookup

0a1e781

refactor: add last sort order id

a550ccb

refactor: add last sort order id and increment

8b09255

chore: add imports

67b9e52

feat: add apply and commit methods

dcaa63f

test: remove spec stuff

ced6a4b

chore: remove unused import

43e09a3

chore: add ReplaceSortOrder to the Transaction class

b460c34

chore: lint

190071f

Merge branch 'apache:main' into feat/update-sort-order

e9475de

kevinjqliu mentioned this pull request Jan 10, 2025

table.update_schema() continues to commit when exist with an exception #1505

Closed

3 tasks

kevinjqliu reviewed Jan 11, 2025

View reviewed changes

JasperHG90 added 5 commits January 24, 2025 10:12

Merge branch 'apache:main' into feat/update-sort-order

eafafaf

chore: renames (replace to update)

ec5f711

chore: renames (replace to update)

d69a071

test: add test updating sort order

b5a5bd8

refactor: remove the sort order builder

8080fa5

JasperHG90 added 5 commits March 23, 2025 20:14

Merge branch 'apache:main' into feat/update-sort-order

2ae47e3

chore: lint and update names

d99dfdd

chore: determine if a sort order is newly added. If so, set the last …

9d77f3f

…assigned id, else set a previous sort order as the default

test: add test for re-using previously defineed sort order

3f6a953

chore: fmt

7223fdd

JasperHG90 commented Mar 24, 2025

View reviewed changes

JasperHG90 added 3 commits March 24, 2025 22:56

chore: only set last assigned order id when sure that a new sort orde…

18ced3f

…r has been added

test: add test for reverting back to unsorted sort order

8f425fb

chore: fmt

f9efab3

JasperHG90 commented Mar 24, 2025

View reviewed changes

JasperHG90 requested a review from kevinjqliu March 27, 2025 09:26

JasperHG90 added 2 commits April 24, 2025 12:18

Merge branch 'apache:main' into feat/update-sort-order

03f0a1b

Merge branch 'apache:main' into feat/update-sort-order

46fd88b

Fokko reviewed May 16, 2025

View reviewed changes

rambleraptor mentioned this pull request Sep 18, 2025

feat(transactions): sort orders #2483

Closed

jayceslesar suggested changes Sep 18, 2025

View reviewed changes

Make the CI happy

353178f

Fokko mentioned this pull request Sep 30, 2025

Feat/update sort order #2552

Merged

kevinjqliu closed this in #2552 Oct 3, 2025

		from pyiceberg.table import Transaction


		class SortOrderBuilder:


		def update_sort_order(self, case_sensitive: bool = True) -> UpdateSortOrder:
		"""Create a new UpdateSortOrder to update the sort order of this table.

Feat: replace sort order #1500

Feat: replace sort order #1500

Uh oh!

Conversation

JasperHG90 commented Jan 8, 2025

Uh oh!

kevinjqliu left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JasperHG90 commented Jan 16, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JasperHG90 commented Apr 15, 2025

Uh oh!

Fokko commented May 16, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JasperHG90 commented May 19, 2025

Uh oh!

mwa28 commented Jul 8, 2025

Uh oh!

Fokko commented Aug 4, 2025

Uh oh!

rambleraptor commented Sep 16, 2025

Uh oh!

jayceslesar left a comment

Choose a reason for hiding this comment

Uh oh!

jayceslesar Sep 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jayceslesar commented Sep 18, 2025

Uh oh!

JasperHG90 commented Sep 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jayceslesar commented Sep 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

kevinjqliu left a comment •

edited

Loading

jayceslesar Sep 18, 2025 •

edited

Loading

JasperHG90 commented Sep 22, 2025 •

edited

Loading

jayceslesar commented Sep 22, 2025 •

edited

Loading