Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[invalidator] fix topic error from date insert #659

Merged
merged 1 commit into from
Dec 31, 2024
Merged

Conversation

stopachka
Copy link
Contributor

@stopachka stopachka commented Dec 31, 2024

I saw a few bugs in our logs, with the error:

class java.time.Instant cannot be cast to class java.lang.Number (java.time.Instant and java.lang.Number are in module java.base of loader 'bootstrap')

Root cause

In datalog, we convert values for checked-data-type = date into java.time.Instant

topicFromQuery [:ea eid aid java.time.Instant<...>]

When we generate topics-for-triple-insert, we keep the underlying number value:

topicFromInvalidator [:ea eid aid 1735658560430]

This means that when we run match on the topic, this causes a type mismatch

Solution

I went ahead and updated topics-for-triple-insert, to convert dates to java.time.Instant

Further work

Right now I realize tests for this flow are a bit cumbersome to write. Noting to look into a future PR, which could make writing something like a) Make a query, b) Make a transaction, c) see that the correct query updates, a bit easier to write

@dwwoelfel @nezaj @tonsky

Copy link

View Vercel preview at instant-www-js-fix-date-iss-jsv.vercel.app.

Copy link
Contributor

@dwwoelfel dwwoelfel left a comment

Choose a reason for hiding this comment

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

Happy new year! LGTM

@stopachka
Copy link
Contributor Author

Happy new year @dwwoelfel !

@stopachka stopachka merged commit 4819517 into main Dec 31, 2024
27 checks passed
@stopachka stopachka deleted the fix-date-iss branch December 31, 2024 16:48
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