Skip to content

fix: omit null LOAD_TABLE_RESPONSE on conditional loadTable after-event - #5638

Merged
jbonofre merged 1 commit into
apache:mainfrom
vigneshio:fix/load-table-304-after-event
Sep 29, 2026
Merged

jbonofre merged 1 commit into
apache:mainfrom
vigneshio:fix/load-table-304-after-event

Conversation

@vigneshio

Copy link
Copy Markdown
Contributor

Conditional loadTable (If-None-Match match → HTTP 304 notModified() with no entity) no longer attaches a null LOAD_TABLE_RESPONSE to the AFTER_LOAD_TABLE event. The persistence event listener also skips null attribute values while pruning, so a null response attribute cannot NPE.

dimas-b
dimas-b previously approved these changes Sep 28, 2026

@dimas-b dimas-b 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.

Nice catch, @vigneshio ! Changes LGTM 👍 Thanks!

@dimas-b
dimas-b requested a review from adutra September 28, 2026 21:53
adutra
adutra previously approved these changes Sep 29, 2026
@jbonofre
jbonofre self-requested a review September 29, 2026 09:13
jbonofre
jbonofre previously approved these changes Sep 29, 2026

@jbonofre jbonofre left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM! Thanks @vigneshio!

Omitting LOAD_TABLE_RESPONSE on HTTP 304 responses while adding the null-safety check in PolarisPersistenceEventListener provides good defense-in-depth.

NB: the CI failure is an unrelated flaky test recently introduced in #5504, I will take a look.

@jbonofre
jbonofre dismissed stale reviews from adutra, dimas-b, and themself via f1d534f September 29, 2026 09:28
@jbonofre
jbonofre force-pushed the fix/load-table-304-after-event branch from f1d534f to 81d8a33 Compare September 29, 2026 09:38
@jbonofre
jbonofre merged commit 098a7b5 into apache:main Sep 29, 2026
24 checks passed
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.

4 participants